QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: low
Invalid

nftPool[tokenId] is not deleted when the tokenId is burned

heavenz52

The nftPool is a public mapping in the UpliftOnlyExample contract . It is a mapping of tokenId to pool address.

It is expected to be deleted or set to zero address when the tokenId is burned as evident from the tests in UpliftOnlyExample.t.sol

function testRemoveLiquidityNegativePriceChange() public {
...
...
...
uint256 nftTokenId = 0;
...
...
assertEq(upliftOnlyRouter.nftPool(nftTokenId), address(0), "pool mapping should be 0");
...
}
​

However this assertion has passed because nftTokenId is initialized with 0 and has never changed it’s value. Since the valid tokenIDs in LPNFT start from 1, the expression upliftOnlyRouter.nftPool(0) will always evaluate to 0.

Recommended Mitigation:

In the function UpliftOnlyExample::onAfterRemoveLiquidity consider burning the NftPool

....
if (feeDataArray[i].amount <= localData.amountLeft) {
uint256 depositAmount = feeDataArray[i].amount;
localData.feeAmount += (depositAmount * feePerLP);
​
localData.amountLeft -= feeDataArray[i].amount;
​
lpNFT.burn(feeDataArray[i].tokenID);
+ nftPool[feeDataArray[i].tokenID] = address(0);
​
delete feeDataArray[i];
feeDataArray.pop();
​
if (localData.amountLeft == 0) {
break;
}
}
...
​
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas / Admin is trusted / Pool creation is trusted / User mistake / Suppositions

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.