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

Lows

inh3l

quantammAdmin not updated when new weight runner is set

Vulnerability Details

In the constructor of the QuantAMMWeightedPool, the quantammAdmin is set by calling the quantammAdmin function of the set updateWeightRunner.

constructor(
NewPoolParams memory params,
IVault vault
) BalancerPoolToken(vault, params.name, params.symbol) PoolInfo(vault) Version(params.version) {
_totalTokens = params.numTokens;
@> updateWeightRunner = UpdateWeightRunner(params.updateWeightRunner);
quantammAdmin = updateWeightRunner.quantammAdmin();
poolRegistry = params.poolRegistry;

However, further down, there's the setUpdateWeightRunnerAddress which allows the quantammAdmin to set a new updateWeightRunner contract.

function setUpdateWeightRunnerAddress(address _updateWeightRunner) external override {
require(msg.sender == quantammAdmin, "ONLYADMIN");
@> updateWeightRunner = UpdateWeightRunner(_updateWeightRunner);
emit UpdateWeightRunnerAddressUpdated(address(updateWeightRunner), _updateWeightRunner);
}

This, however, and coupled with the fact that the quantammAdmin parameter is marked as immutable, means that the quantammAdmin is not (and cannot) updated when a new updateWeightRunner is set.

address internal immutable quantammAdmin;

The new updateWeightRunner's quantamm admin has no control over the QuantAMMWeightedPool. If the new updateWeightRunner is set due to emergency actions, the new admin is locked out, while the old admin can still make changes.

Recommendations

Change the imutability status of the quantammAdmin parameter and set the new quantammAdmin in the setUpdateWeightRunnerAddress function.

Use of a general oracleStalenessThreshold parameter

Vulnerability Details

QuantAMMWeightedPool.sol's initialization allows setting the oracle staleness threshold.

function initialize(
int256[] memory _initialWeights,
PoolSettings memory _poolSettings,
int256[] memory _initialMovingAverages,
int256[] memory _initialIntermediateValues,
uint _oracleStalenessThreshold
) public initializer {
//...
​
@> oracleStalenessThreshold = _oracleStalenessThreshold;
updateInterval = _poolSettings.updateInterval;
_setRule(_initialWeights, _initialIntermediateValues, _initialMovingAverages, _poolSettings);
​
_setInitialWeights(_initialWeights);
}
​

Also, there's no function to update this parmaeter, hence the same staleness threshold is used for all oracle feeds being queried by the update weight runner.

@> uint oracleStalenessThreshold = IQuantAMMWeightedPool(_pool).getOracleStalenessThreshold();
​
for (uint i; i < oracleLength; ) {
// Asset is base asset
OracleData memory oracleResult;
oracleResult = _getOracleData(OracleWrapper(optimisedOracles[i]));
@> if (oracleResult.timestamp > block.timestamp - oracleStalenessThreshold) {
outputData[i] = oracleResult.data;
} else {
unchecked {
numAssetOracles = poolBackupOracles[_pool][i].length;
}
​
for (uint j = 1 /*0 already done via optimised poolOracles*/; j < numAssetOracles; ) {
oracleResult = _getOracleData(
// poolBackupOracles[_pool][asset][oracle]
OracleWrapper(poolBackupOracles[_pool][i][j])
);
@> if (oracleResult.timestamp > block.timestamp - oracleStalenessThreshold) {
// Oracle has fresh values
break;
} else if (j == numAssetOracles - 1) {
// All oracle results for this data point are stale. Should rarely happen in practice with proper backup oracles.
​
revert("No fresh oracle values available");
}
unchecked {
++j;
}
}
outputData[i] = oracleResult.data;
}

This can lead to a number of issues when querying prices. For instance, if the price feed is one that is slow to update e.g (most LSTs/ETH feeds), after a certain point, their price become unqueriable because the currently set threshold will be too small for their last update time, if its one that updates very fast, stale prices may be allowed if the oracle for some reason fails to update. This issue is also excarcebated by the fact that the contrracts are to be deployed accross various chains where these thresholds may be different.

Recommendations

Use the ruleOracleStalenessThreshold mapping instead to set the staleness threshold for each oracle feed in use.

Excess deposit poolsFeeData restriction can be bypassed through transfers

Vulnerability Details

When a user adds liquidity, there's a restriction to ensure that the user doesn't have more than 100 poolFeeData attached to their address.

if (poolsFeeData[pool][msg.sender].length > 100) {
revert TooManyDeposits(pool, msg.sender);
}

This restriction is however not placed in the afterUpdate hook, which is called during lpNFT transfers which also pushes a new poolsFeeData into the user's array.

This makes the restriction ineffective in the attempts to protect against DDoS attacks.

Recommendations

Enforce the restriction in the afterUpdate hook too.

Updates

Lead Judging Commences

n0kto Lead Judge 10 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.