theweb3mechanic - Gas refund is unfairly calculated during pause time

[sherlock-admin2]

theweb3mechanic

Medium

Gas refund is unfairly calculated during pause time

Summary

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

Vulnerability Detail

The logic responsible for calculating fees and refunds is based on the difference between the expected listing duration and the current time (block.timestamp) at the moment the function is called. This refund is intended to compensate for the unused listing days. However, the value of the refund decreases over time, leading to the listing owner eventually paying more than expected in taxes, as the paused duration is not accounted for in the calculation.

Impact

Pause periods are included in the calculation of listing days, which unfairly impacts users' gas refunds when performing actions such as modifyListing and cancelListing.

Code Snippet

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

    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;
        }

Tool used

Manual Review

Recommendation

Consider accounting for pause time when calculating refunds. However, this must be implemented carefully to avoid introducing new bugs into the logic. The pause time should not be exploitable and must be calculated based on the total pause duration experienced from the moment the listing was created until the time of the function call.

            refund_ = (_listing.duration - (block.timestamp - _listing.created) - pauseduration) * taxPaid / _listing.duration;