QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: medium
Valid

poolsFeeData[pool][_to]'s length is not checked when afterUpdate()

zhanmingjing

Summary

poolsFeeData[pool][_to]'s length is not checked when afterUpdate()

Vulnerability Details

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L614

poolsFeeData[pool][user]' length should not be over 100. When addLiquidityProportional(), poolsFeeData[pool][user]' length is checked.

However when afterUpdate(), transfer to "address _to", poolsFeeData[poolAddress][to].length is not checked, which could cause poolsFeeData[poolAddress][_to].length to be over 100. Please refer to the following steps:

1, poolsFeeData[pool][addressA]' length is 100.

2, poolsFeeData[pool][addressB]' length is 60.

3, call afterUpdate() mutiple times, transfer addressB's all 60 tokens to addressA.

4, poolsFeeData[pool][addressA]' length will be 160.

Impact

Some of poolsFeeData[pool][user]'s length will be over 100.

Tools Used

manually reviewed

Recommendations

  1. check the poolsFeeData at the quite beginning of function afterUpdata()

  2. when addLiquidityProportional(), seems better to change "if (poolsFeeData[pool][msg.sender].length > 100)" to "if (poolsFeeData[pool][msg.sender].length >= 100)"

function afterUpdate(address _from, address _to, uint256 _tokenID) public {
if (msg.sender != address(lpNFT)) {
revert TransferUpdateNonNft(_from, _to, msg.sender, _tokenID);
}
​
address poolAddress = nftPool[_tokenID];
​
if (poolAddress == address(0)) {
revert TransferUpdateTokenIDInvaid(_from, _to, _tokenID);
}
if (poolsFeeData[poolAddress][_to].length >= 100) {
revert TooManyDeposits(poolAddress, to);
}
......
}
Updates

Lead Judging Commences

n0kto Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_afterUpdate_does_not_check_limit_NFT_per_user

Likelihood: Medium/High, anyone can receive an unlimited NFT number but will cost creation of LP tokens and sending them. Impact: Low/Medium, DoS the afterUpdate and addLiquidityProportional but will be mitigable on-chain because a lot of those NFT can be burn easily in onAfterRemoveLiquidity.

Support

FAQs

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