merlinboii - Incorrect tax accounting due to failure in handling liquidation listings in `Listings::relist()`

[sherlock-admin2]

merlinboii

Medium

Incorrect tax accounting due to failure in handling liquidation listings in Listings::relist()

Summary

The Listings::relist() function fails to correctly manage liquidation listings, which do not pay taxes and should not be included in tax calculations. However, the current implementation calculates refunds based on the floor, leading to improper tax usage and incorrect accounting.

Vulnerability Detail

Liquidation listings, created through the Listings::createLiquidationListing() function, are not taxed like normal listings. Regular listings prepay taxes that are used to spend as fees for the UniswapImplementation. If the listing period ends early, users are refunded the unused portion of prepaid taxes.

However, liquidation listings should be exempt from tax calculations and refunds, but the Listings::relist() function does not properly differentiate between liquidation and regular listings.

This issue arises because the refund is calculated using the floor value, even for liquidation listings that are not subject to taxes. As a result, liquidation listings mistakenly receive tax refunds, which leads to incorrect accounting and fee distribution.

Impact

Incorrect tax is being accounted to liquidation listings, and funds meant for non-liquidation listings are being used improperly. This leads to eligible listings losing part of their prepaid taxes during the relisting process of liquidation listings.

Code Snippet

Listings::relist()

File: Listings.sol
625:     function relist(CreateListing calldata _listing, bool _payTaxWithEscrow) public nonReentrant lockerNotPaused {
---
643:         // We can process a tax refund for the existing listing
644:@>       (uint _fees,) = _resolveListingTax(oldListing, _collection, true);
645:         if (_fees != 0) {
646:             emit ListingFeeCaptured(_collection, _tokenId, _fees);
647:         }
---
672:     }

Listings::_resolveListingTax()

File: Listings.sol
918:     function _resolveListingTax(Listing memory _listing, address _collection, bool _action) private returns (uint fees_, uint refund_) {
919:         // If we have been passed a Floor item as the listing, then no tax should be handled
920:         if (_listing.owner == address(0)) {
921:             return (fees_, refund_);
922:         }
923: 
924:         // Get the amount of tax in total that will have been paid for this listing
925:         uint taxPaid = getListingTaxRequired(_listing, _collection);
926:         if (taxPaid == 0) {
927:             return (fees_, refund_);
928:         }
929: 
930:         // Get the amount of tax to be refunded. If the listing has already ended
931:         // then no refund will be offered.
932:         if (block.timestamp < _listing.created + _listing.duration) {
933:             refund_ = (_listing.duration - (block.timestamp - _listing.created)) * taxPaid / _listing.duration;
934:         }
935: 
936:         // Send paid tax fees to the {FeeCollector}
937:         unchecked {
938:             fees_ = (taxPaid > refund_) ? taxPaid - refund_ : 0;
939:         }
940: 
941:         if (_action) {
942:             ICollectionToken collectionToken = locker.collectionToken(_collection);
943: 
944:             if (fees_ != 0) {
945:                 IBaseImplementation implementation = locker.implementation();
946: 
947:                 collectionToken.approve(address(implementation), fees_);
948:                 implementation.depositFees(_collection, 0, fees_);
949:             }
950: 
951:             // If there is tax to refund, then allocate it to the user via escrow
952:             if (refund_ != 0) {
953:                 _deposit(_listing.owner, address(collectionToken), refund_);
954:             }
955:         }
956:     }

Tool used

Manual Review

Recommendation

Two possible approaches can be taken depending on the desired design:

1. Allow relisting of liquidation listings:

In this approach, ensure that liquidation listings are relisted but do not participate in tax refund calculations or fee distributions since they are exempt from taxes.

File: Listings.sol

function relist(CreateListing calldata _listing, bool _payTaxWithEscrow) public nonReentrant lockerNotPaused {
---

-    // We can process a tax refund for the existing listing
+    // We can process a tax refund for the existing listing if it isn't a liquidation
+   if (!_isLiquidation[_collection][_tokenId]) {
        (uint _fees,) = _resolveListingTax(oldListing, _collection, true);
        if (_fees != 0) {
            emit ListingFeeCaptured(_collection, _tokenId, _fees);
        }
+   }

---
}

2. Do not allow relisting of liquidation listings:

In this approach, prevent liquidation listings from being relisted by reverting the transaction when an attempt to relist is made.

function relist(uint _listingId, uint _newDuration) external returns (uint listingId) {
---

+   // Revert if attempting to relist a liquidation listing
+   if (_isLiquidation[_collection][_tokenId]) {
+       revert NotAllowLiquidationListing;
+   }

---
}