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

Lack of Reentrancy Protection

mibrobuba

Summary

The _setRule function lacks explicit reentrancy protection mechanisms, such as the nonReentrant modifier or similar safeguards.This absence creates potential vulnerabilities, especially given its reliance on:

External Calls:

  • _poolSettings.rule.initialisePoolRuleIntermediateValues is an external call, which might invoke untrusted or malicious contracts.

  • The setRuleForPool function in updateWeightRunner is another external call, exposing the system to reentrancy if the UpdateWeightRunner contract is compromised or implemented insecurely.

State Updates:

  • Critical state variables (lambda, epsilonMax, absoluteWeightGuardRail, etc.) are updated after external calls. This sequencing leaves the contract in an inconsistent state during those calls, making it susceptible to exploitation.

If a malicious contract gains control during an external call:

  1. Intermediate State Manipulation:

    • It could trigger reentrant calls to manipulate state variables or force invalid configurations, such as bypassing require checks.

  2. Oracle Exploitation:

    • Malicious oracles could exploit their integration via _poolSettings.rule.initialisePoolRuleIntermediateValues to provide manipulated data.

Vulnerability Details

function _setRule(
int256[] memory _initialWeights,
int256[] memory _ruleIntermediateValues,
int256[] memory _initialMovingAverages,
IQuantAMMWeightedPool.PoolSettings memory _poolSettings
) internal {
require(address(_poolSettings.rule) != address(0), "Invalid rule");
​
​
for (uint i; i < _poolSettings.lambda.length; ++i) {
int256 currentLambda = int256(uint256(_poolSettings.lambda[i]));
require(currentLambda > PRBMathSD59x18.fromInt(0) && currentLambda < PRBMathSD59x18.fromInt(1), "INVLAM"); //Invalid lambda value
}
​
​
require(
_poolSettings.lambda.length == 1 || _poolSettings.lambda.length == _initialWeights.length,
"Either scalar or vector"
);
int256 currentEpsilonMax = int256(uint256(_poolSettings.epsilonMax));
require(
currentEpsilonMax > PRBMathSD59x18.fromInt(0) && currentEpsilonMax <= PRBMathSD59x18.fromInt(1),
"INV_EPMX"
); //Invalid epsilonMax value
​
​
//applied both as a max (1 - x) and a min, so it cant be more than 0.49 or less than 0.01
//all pool logic assumes that absolute guard rail is already stored as an 18dp int256
require(
int256(uint256(_poolSettings.absoluteWeightGuardRail)) <
PRBMathSD59x18.fromInt(1) / int256(uint256((_initialWeights.length))) &&
int256(uint256(_poolSettings.absoluteWeightGuardRail)) >= 0.01e18,
"INV_ABSWGT"
); //Invalid absoluteWeightGuardRail value
​
​
require(_poolSettings.oracles.length > 0, "NOPROVORC"); //No oracle indices provided
require(_poolSettings.rule.validParameters(_poolSettings.ruleParameters), "INVRLEPRM"); //Invalid rule parameters
​
​
//0 is hodl, 1 is trade whole pool which invariant doesnt let you do anyway
require(_poolSettings.maxTradeSizeRatio > 0 && _poolSettings.maxTradeSizeRatio <= 0.3e18, "INVMAXTRADE"); //Invalid max trade size
​
​
lambda = _poolSettings.lambda;
epsilonMax = _poolSettings.epsilonMax;
absoluteWeightGuardRail = _poolSettings.absoluteWeightGuardRail;
maxTradeSizeRatio = _poolSettings.maxTradeSizeRatio;
​
​
ruleParameters = _poolSettings.ruleParameters;
​
​
_poolSettings.rule.initialisePoolRuleIntermediateValues(
address(this),
_initialMovingAverages,
_ruleIntermediateValues,
_initialWeights.length
);
​
​
updateWeightRunner.setRuleForPool(_poolSettings);
}

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/QuantAMMWeightedPool.sol#L756-L809

Impact

  • It could trigger reentrant calls to manipulate state variables or force invalid configurations, such as bypassing require checks.

  • Malicious oracles could exploit their integration via _poolSettings.rule.initialisePoolRuleIntermediateValues to provide manipulated data.

  • Reentrancy attacks can compromise the contract's integrity.

Tools Used

  • Manual Code Review

Recommendations

  • Use OpenZeppelin’s ReentrancyGuard: Add the ReentrancyGuard library to the contract and apply the nonReentrant modifier to _setRule.

contract QuantAMMWeightedPool is ReentrancyGuard {
...
function _setRule(...) internal nonReentrant {
...
}
}
​

Reorder State Updates and External Calls: Perform state updates before making external calls:

function _setRule(...) internal nonReentrant {
require(address(_poolSettings.rule) != address(0), "Invalid rule");
...
​
// Update state variables first
lambda = _poolSettings.lambda;
epsilonMax = _poolSettings.epsilonMax;
absoluteWeightGuardRail = _poolSettings.absoluteWeightGuardRail;
maxTradeSizeRatio = _poolSettings.maxTradeSizeRatio;
ruleParameters = _poolSettings.ruleParameters;
​
// Perform external calls after internal state is finalized
_poolSettings.rule.initialisePoolRuleIntermediateValues(
address(this),
_initialMovingAverages,
_ruleIntermediateValues,
_initialWeights.length
);
​
updateWeightRunner.setRuleForPool(_poolSettings);
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Too generic
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.

Give us feedback!