Incorrect use of totalPendingTokens leads to inability to rescue ERC-1155 tokens
Description
Admins can rescue ERC-1155 tokens in LockedUSDzMarket via rescueERC1155(), and there is a check to ensure they cannot rescue tokens currently reserved for pending orders.
function rescueERC1155(uint256 tokenId, address to, uint256 amount) external onlyRole(DEFAULT_ADMIN_ROLE) {
require(amount <= lockedUsdz.balanceOf(address(this), tokenId) - totalPendingTokens, "Cannot rescue ERC1155 tokens reserved for pending offers");
lockedUsdz.safeTransferFrom(address(this), to, tokenId, amount, "");
}However, the use of totalPendingTokens is incorrect here. The totalPendingTokens is the amount of pending tokens for orders, for all token IDs combined. If there are pending orders that have a different token ID than the token ID to rescue, admins will be unable to rescue all lost tokens.
Impact
Admins will likely be unable to rescue any ERC-1155 tokens lost in the contract.
For example, if there is a pending order with 100 tokens of ID 1, and 50 tokens of ID 2 need to be rescued, totalPendingTokens will be 100, while lockedUsdz.balanceOf(address(this), 2) will be 50, resulting in admins being unable to rescue any tokens of ID 2 due to underflow.
function testAuditUnableRescueERC1155() public {
// create offer for 100 tokens of Id 1
uint256 tokenId = 1;
uint256 offerAmount = 100e18;
uint256 askPrice = 0.99e6;
vm.prank(user1);
market.createOffer(tokenId, offerAmount, askPrice);
// someone loses 50 tokens of Id 2
uint256 rescueTokenId = 2;
uint256 rescueAmount = 50e18;
lockedUsdz.mint(address(market), rescueTokenId, rescueAmount);
// admin tries rescue 50 tokens of Id 2
vm.prank(admin);
market.rescueERC1155(2, admin, rescueAmount); // fails due to underflow
assertEq(lockedUsdz.balanceOf(admin, rescueTokenId), rescueAmount);
}The following text is the result of the proof-of-concept script, which fails, showing the admin is unable to rescue the tokens:
Ran 1 test for test/LockedUSDzMarketTest.t.sol:LockedUsdzMarketTest
[FAIL. Reason: panic: arithmetic underflow or overflow (0x11)] testAuditUnableRescueERC1155() (gas: 308482)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 8.13ms (1.13ms CPU time)Recommendations
Use a mapping to store pending tokens for each token ID, such as mapping(uint256 => uint256) public pendingTokens;.
Remediation
This issue has been acknowledged by Anzen Labs Inc., and a fix was implemented in commit d20afe58↗.