almantare - Relist Liquidation Listing lead to paying tx from protocol - loss 0.0514 collectionToken

[sherlock-admin4]

almantare

High

Relist Liquidation Listing lead to paying tx from protocol - loss 0.0514 collectionToken

Summary

In the Listings contract, there are two ways to create a listing.

1. By calling the Listings::createListings function. Paying a fee for each Listing. (The function can be called by any address)
2. By calling the Listings::createLiquidationListing function. This function is only called from the ProtectedListings::liquidateProtectedListing function. No fee is paid for creating the listing in this case. Also, an entry is created in the _isLiquidation dictionary. The listing is marked as true.

As a result of both functions, the created listing is added to the _listings dictionary.

For this vulnerability, it's important to understand the distribution of fees paid for creating a listing. The full logic can be found in the _resolveListingTax function

Simplified, this logic looks like this: tx is divided into two components. Refund and Fees. When purchased, the Refund is sent back to the user who created the Listing, and the fees are sent to the UniswapPool. The faster the fill / modify / relist listing, the closer the refund is to the initial tx. If the collection is bought after the duration expires ⇒ refund = 0, fees = tx

However, the relist function calls the _resolveListingTax function with the parameter bool _action = true, which means that the fee payment will be made directly within the function. However, when distributing the funds for fees in this function, there is no validation that the original listing should be _isLiquidation = false, otherwise we are distributing a fee for a listing that no one paid for, which means we are simply spending protocol funds.

Vulnerability Detail

The root cause of the vulnerability is that the _resolveListingTax function does not check whether we are withdrawing a commission for a liquidationListing or not. In other places where the commission is withdrawn, the protocol performs this check [Listings::_fillListings](https://github.com/sherlock-audit/2024-08-flayer-BengalCatBalu/blob/d53f757bf5daac1e9d0c38a649d74efd9195494e/flayer/src/contracts/Listings.sol#L501-L501) [Listings::reserve](https://github.com/sherlock-audit/2024-08-flayer-BengalCatBalu/blob/d53f757bf5daac1e9d0c38a649d74efd9195494e/flayer/src/contracts/Listings.sol#L714-L719).

Calls to _resolveListingTax with the parameter action = true without a preliminary check for isLiquidationListing = false occur only in relist.

Let's calculate how much the protocol loses in this case.

The commission for placing a liquidationListing with parameters floor_multiple = 400, duration = 4 days can be calculated by running this test.

function test_CalculateTax() public {
        console.logUint(taxCalculator.calculateTax(address(0), 400, 4  days));
}

In total, the protocol will lose: 0.05142857142857143 collectionToken. Considering that the protocol logic assumes that 1 token = 1 floor price for nft in eth - this loss will be significant, especially for expensive collections (5% of the floor price).

Impact

For this error to occur and for the protocol to lose ~0.05 tokens, someone needs to decide to call relist from a liquidationListing. Since relist can be called by any user except listing.owner, the probability of this is high.

The protocol's loss is also substantial, considering that it can occur frequently, including on expensive collections.

Severity: High

Code Snippet

https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/Listings.sol#L918-L956

Tool used

Manual Review

Recommendation

Add isLiquidation check to relist function