Audinarey - Users cannot unlock/repay their protected listing when the Locker is paused

[sherlock-admin2]

Audinarey

High

Users cannot unlock/repay their protected listing when the Locker is paused

Summary

Users cannot repay or unlock their protected listing when Locker is paused, this can lead to liquidation of their protected asset leading to a loss for an honest user

Root Cause

unlockProtectedListing(...) is used to repay a protected listing's position and optionally redeem their token immediately. However, the problem is that users are blocked from making this repayment when the Locker is locked leading to a possible liquidation of the user and I beleive this is a design flaw considering that there is an option for the user to repay and let their asset remain in the Locker for later withdrawal.

File: ProtectedListings.sol
287: @>  function unlockProtectedListing(address _collection, uint _tokenId, bool _withdraw) public lockerNotPaused {
288:         // Ensure this is a protected listing
289:         ProtectedListing memory listing = _protectedListings[_collection][_tokenId];
290: 
291:         // Ensure the caller owns the listing
292:         if (listing.owner != msg.sender) revert CallerIsNotOwner(listing.owner);
293: 

Internal pre-conditions

Locker is paused

External pre-conditions

No response

Attack Path

• Alice wants to repay her loan that is on the verge of becoming unhealthy
• Locker is paused
• Alice call reverts
• By the time locker is unpaused, her position has become unhealthy and is due for liquidation

Impact

Users canot repay when Locker is paused leading to liquidation of their protected listings

PoC

No response

Mitigation

Consider modifying the unlockProtectedListing(...) as shown below

File: ProtectedListings.sol
-287:     function unlockProtectedListing(address _collection, uint _tokenId, bool _withdraw) public lockerNotPaused {
+287:     function unlockProtectedListing(address _collection, uint _tokenId) public {
288:         // Ensure this is a protected listing
289:         ProtectedListing memory listing = _protectedListings[_collection][_tokenId];
290: 
291:         // Ensure the caller owns the listing
292:         if (listing.owner != msg.sender) revert CallerIsNotOwner(listing.owner);
293: 
294:         // Ensure that the protected listing has run out of collateral
295:         int collateral = getProtectedListingHealth(_collection, _tokenId); // @audit 5) I think it should be <= 0 as zero also signifies undercollateralization
296:         if (collateral < 0) revert InsufficientCollateral(); // @audit 1) if a user uses a small amount say listing.tokenTaken = 1 wei, then collateral will always be > 0
297: 
298:         // cache
299:         ICollectionToken collectionToken = locker.collectionToken(_collection);
300:         uint denomination = collectionToken.denomination();
301:         uint96 tokenTaken = _protectedListings[_collection][_tokenId].tokenTaken;
302: 
303:         // Repay the loaned amount, plus a fee from lock duration
304:         uint fee = unlockPrice(_collection, _tokenId) * 10 ** denomination; // full debt @audit user can unlock protected without paying fees??
305:         collectionToken.burnFrom(msg.sender, fee); // @audit 2) no amount will be burnt if significantly small amount is used for listing.tokenTaken
306: 
307:         // We need to burn the amount that was paid into the Listings contract @audit 4) even the amount burned is calculated wrong
308:         collectionToken.burn((1 ether - tokenTaken) * 10 ** denomination); // @audit 3) tokens are burned from Plisting contract instead of Plisting owner check that after unlocking user still has token and tokenId has been returned to him
309:         // @audit 6) what if this contract does not have enough tokens to burn say for the first repayer
310:         // Remove our listing type @audit 7) the above burning takes a toll on the UR especially if a user borrow dust amount more of the total supply is burnt thus incresing the UR
311:         unchecked { --listingCount[_collection]; } // @audit hence user can take out dust amount (say 1 wei of coll token) continuously and unlock continuously causing the UR to rise rapidly and by extension the interest rate without actually this can lead to a liquidation of other positions because they will get to liquidation faster especially for positions that have 
312: 
313:         // Delete the listing objects
314:         delete _protectedListings[_collection][_tokenId];
315: 
316:         // Transfer the listing ERC721 back to the user
-317:         if (_withdraw) {
+317:         if (!locker.paused()) {
318:             locker.withdrawToken(_collection, _tokenId, msg.sender);
319:             emit ListingAssetWithdraw(_collection, _tokenId);
320:         } else {
321:             canWithdrawAsset[_collection][_tokenId] = msg.sender;
322:         }
323: 
324:         // Update our checkpoint to reflect that listings have been removed
325:         _createCheckpoint(_collection);
326: 
327:         // Emit an event
328:         emit ListingUnlocked(_collection, _tokenId, fee);
329:     }