0xAristos - "Improper Handling of Pause Periods in Listing Refund Calculation Leading to Excessive Ta

[sherlock-admin4]

0xAristos

Medium

"Improper Handling of Pause Periods in Listing Refund Calculation Leading to Excessive Ta

Summary

The listings::cancelListings and listings::modifyListings functions utilize the lockerNotPause modifier to ensure that these functions cannot be executed when the contract is paused, which is designed to handle emergency situations. However, this functionality is not properly implemented, as both functions still make internal calls to __resolveListingTax, which calculates associated fees and gas refunds.

Vulnerability Detail

The fee and refund calculation logic is based on the difference between the expected listing duration and the current time (block.timestamp) at the time of function execution. This calculation is meant to refund users for the remaining unused listing period. However, since the pause period is not factored into the calculation, the value of the refund decreases over time. As a result, the listing owner could end up paying more in taxes than expected, because the duration during which the contract was paused is not considered.

Impact
The inclusion of pause periods in the calculation of listing days unfairly reduces users' gas refunds when executing modifyListing and cancelListing functions, leading to overpayment in taxes.

##Code Snippet
https://github.com/sherlock-audit/2024-08-flayer/blob/0ec252cf9ef0f3470191dcf8318f6835f5ef688c/flayer/src/contracts/Listings.sol#L932

Copy code
function _resolveListingTax(Listing memory _listing, address _collection, bool _action) private returns (uint fees_, uint refund_) {
    // If we have been passed a Floor item as the listing, then no tax should be handled
    if (_listing.owner == address(0)) {
        return (fees_, refund_);
    }

    // Get the amount of tax in total that will have been paid for this listing
    uint taxPaid = getListingTaxRequired(_listing, _collection);
    if (taxPaid == 0) {
        return (fees_, refund_);
    }

    // Get the amount of tax to be refunded. If the listing has already ended
    // then no refund will be offered.
    if (block.timestamp < _listing.created + _listing.duration) {
        //@audit pause time is not considered
        refund_ = (_listing.duration - (block.timestamp - _listing.created)) * taxPaid / _listing.duration;
    }
}

Code reference

##Tool Used
Manual Review

Recommendation

To prevent this issue, consider incorporating the pause duration when calculating refunds. However, care must be taken to ensure this implementation does not introduce new vulnerabilities. The pause duration should be calculated based on the total time the contract was paused, from the listing’s creation until the time the function is called.

Copy code
refund_ = (_listing.duration - (block.timestamp - _listing.created) - pauseDuration) * taxPaid / _listing.duration;

This approach ensures that pause periods are excluded from the listing duration and that users receive a fair refund based on the actual active listing time.