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

Inconsistency in Pool Registry Mask Handling (Function: setWeightsManually

mibrobuba

Summary

The function retrieves the poolRegistryEntry using QuantAMMWeightedPool(_poolAddress).poolRegistry(), but it does not validate whether the _poolAddress corresponds to a legitimate, pre-approved pool in the QuantAMM system. an attacker may supply a malicious or manipulated _poolAddress, the mask checks (MASK_POOL_OWNER_UPDATES, MASK_POOL_QUANTAMM_ADMIN_UPDATES, etc.) may inadvertently grant unauthorized access to weight updates..

Vulnerability Details

function setWeightsManually(
int256[] calldata _weights,
address _poolAddress,
uint40 _lastInterpolationTimePossible,
uint _numberOfAssets
) external {
uint256 poolRegistryEntry = QuantAMMWeightedPool(_poolAddress).poolRegistry();
if (poolRegistryEntry & MASK_POOL_OWNER_UPDATES > 0) {
require(msg.sender == poolRuleSettings[_poolAddress].poolManager, "ONLYMANAGER");
} else if (poolRegistryEntry & MASK_POOL_QUANTAMM_ADMIN_UPDATES > 0) {
require(msg.sender == quantammAdmin, "ONLYADMIN");
} else {
revert("No permission to set weight values");
}
​
​
//though we try to keep manual overrides as open as possible for unknown unknows
//given how the math library works weights it is easiest to define weights as 18dp
//even though technically G3M works of the ratio between them so it is not strictly necessary
//CYFRIN L-02
for (uint i; i < _weights.length; i++) {
if (i < _numberOfAssets) {
require(_weights[i] > 0, "Negative weight not allowed");
require(_weights[i] < 1e18, "greater than 1 weight not allowed");
}
}
​
​
IQuantAMMWeightedPool(_poolAddress).setWeights(_weights, _poolAddress, _lastInterpolationTimePossible);
​
​
emit SetWeightManual(msg.sender, _poolAddress, _weights, _lastInterpolationTimePossible);
}
​
​
/// @notice Breakglass function to allow the admin or the pool manager to set the intermediate values of the rule manually
/// @param _poolAddress the target pool
/// @param _newMovingAverages manual new moving averages
/// @param _newParameters manual new parameters
/// @param _numberOfAssets number of assets in the pool

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L559-L594

Impact

  • Malicious actors could create pools with spoofed or manipulated registry entries to bypass access control.

  • unauthorized users could potentially bypass the permission checks by using a pool with manipulated registry values.

  • Allowing unauthorized updates to weights could destabilize the pool and compromise the protocol’s security and reliability.

Tools Used

  • Manual Code Review

Recommendations

Maintain a Registry of Approved Pools: Introduce a mapping(address => bool) to track approved pool addresses and ensure _poolAddress is validated against this list before proceeding.

mapping(address => bool) public approvedPools;
​
// Add a function for the admin to approve pools
function approvePool(address _poolAddress) external onlyOwner {
require(_poolAddress != address(0), "Invalid pool address");
approvedPools[_poolAddress] = true;
}
​
function revokePool(address _poolAddress) external onlyOwner {
require(approvedPools[_poolAddress], "Pool not approved");
approvedPools[_poolAddress] = false;
}
​

  1. Validate _poolAddress in setWeightsManually: Before proceeding with registry mask checks, ensure _poolAddress is part of the approved pools.

require(approvedPools[_poolAddress], "Pool address not approved");

  1. **setWeightsManually **Here’s a corrected implementation:

function setWeightsManually(
int256[] calldata _weights,
address _poolAddress,
uint40 _lastInterpolationTimePossible,
uint _numberOfAssets
) external {
// Validate pool address
require(approvedPools[_poolAddress], "Pool address not approved");
​
// Retrieve pool registry entry
uint256 poolRegistryEntry = QuantAMMWeightedPool(_poolAddress).poolRegistry();
​
// Access control based on registry mask
if (poolRegistryEntry & MASK_POOL_OWNER_UPDATES > 0) {
require(msg.sender == poolRuleSettings[_poolAddress].poolManager, "ONLYMANAGER");
} else if (poolRegistryEntry & MASK_POOL_QUANTAMM_ADMIN_UPDATES > 0) {
require(msg.sender == quantammAdmin, "ONLYADMIN");
} else {
revert("No permission to set weight values");
}
​
// Validate weights
for (uint i; i < _weights.length; i++) {
if (i < _numberOfAssets) {
require(_weights[i] > 0, "Negative weight not allowed");
require(_weights[i] < 1e18, "Weight greater than 1 not allowed");
}
}
​
// Update weights in the pool
IQuantAMMWeightedPool(_poolAddress).setWeights(_weights, _poolAddress, _lastInterpolationTimePossible);
​
emit SetWeightManual(msg.sender, _poolAddress, _weights, _lastInterpolationTimePossible);
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
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!