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

lack of upliftFeeBps update function.

0xwsecteam

Summary

The UpliftOnlyExample contract defines a public variable upliftFeeBps, but does not provide any function to update its value, but the contract give strong evidance that this value should be updated.

Vulnerability Details

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

The variable upliftFeeBps represents a fee value in basis points (BPS), which is used in the contract to calculate feePerLP.
However, the contract lacks a function to update the upliftFeeBps value after deployment.

There is evidence in the code indicating that upliftFeeBps is intended to be updatable. Specifically, when a user transfers an NFT,
the line feeDataArray[tokenIdIndex].upliftFeeBps is updated with the current value of upliftFeeBps.
However, since no function exists to update the upliftFeeBps value, this line of code has no practical effect. It highlights that upliftFeeBps is designed to be modifiable,
yet the absence of an update mechanism renders this logic non functional.

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

function afterUpdate(address _from, address _to, uint256 _tokenID) public {
// snip...
if (tokenIdIndexFound) {
if (_to != address(0)) {
// Update the deposit value to the current value of the pool in base currency (e.g. USD) and the block index to the current block number
// vault.transferLPTokens(_from, _to, feeDataArray[i].amount);
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
|>>> feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
​
//actual transfer not a afterTokenTransfer caused by a burn
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]);
​
if (tokenIdIndex != feeDataArrayLength - 1) {
//Reordering the entire array could be expensive but it is the only way to keep true FILO
for (uint256 i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
delete feeDataArray[i - 1];
feeDataArray[i - 1] = feeDataArray[i];
}
}
​
delete feeDataArray[feeDataArrayLength - 1];
feeDataArray.pop();
}
}
}

If was not intended to be updatable then no need to update upliftFeeBps here.

Impact

  • Without an update mechanism, the contract is unable to reflect changes in fee structures.

  • Fee structures that remain static in a dynamic market environment may lead to a mismatch between the protocol's offerings and market expectations, reducing competitiveness.

Recommendations

File: https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol
Add a function to update the upliftFeeBps variable, controlled by appropriate access restrictions to ensure security. Below is a sample implementation:

// Function to update upliftFeeBps
function setUpliftFeeBps(uint64 _newFeeBps) external onlyOwner {
require(_newFeeBps <= 10000, "Fee exceeds maximum BPS"); // Ensure fee is valid (<= 100%)
upliftFeeBps = _newFeeBps;
emit UpliftFeeBpsUpdated(_newFeeBps);
}
Updates

Lead Judging Commences

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

invalid_immutable_oracles/variables

Appeal created

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

finding_upliftFeeBps_is_immutable_but_should_be_changeable_according_to_the_sponsor

Likelihood: Low, it cannot be changed but should not need to be changed often. Impact: Low, the code still works with fees.

Support

FAQs

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