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

Admin Cannot Call Restricted Functions if Both Admin and Owner Are Permitted

mahdirostami

Summary

In InitialisePoolLastRunTime and setIntermediateValuesManually and setWeightsManually, if both the admin and the owner are permitted to perform restricted actions via MASK_POOL_OWNER_UPDATES and MASK_POOL_QUANTAMM_ADMIN_UPDATES, calls made by the admin will revert due to the precedence of the owner check.

Vulnerability Details

The issue lies in the conditional checks within the functions InitialisePoolLastRunTime and setIntermediateValuesManually and setWeightsManually:

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 last run time");
}

If both MASK_POOL_OWNER_UPDATES and MASK_POOL_QUANTAMM_ADMIN_UPDATES are enabled in poolRegistryEntry, the first condition (MASK_POOL_OWNER_UPDATES) takes precedence. This causes the contract to expect the sender to be the owner (poolManager) and results in a revert ("ONLYMANAGER") when the admin attempts to call these functions.

Impact

Admin cannot perform permitted actions when both admin and owner are granted access.

Proof of Concept

The following test demonstrates the issue:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import "../../contracts/mock/MockUpdateWeightRunner.sol"; // Update with your actual path
import "../../contracts/mock/MockQuantAMMBasePool.sol"; // Update with your actual path
​
contract UpdateWeightRunnerTest is Test {
MockUpdateWeightRunner internal updateWeightRunner;
address internal owner;
address internal admin;
address internal addr2;
​
MockQuantAMMBasePool mockPool;
​
function setUp() public {
(address ownerLocal, address addr1Local, address addr2Local) = (vm.addr(1), vm.addr(2), vm.addr(3));
owner = ownerLocal;
admin = addr1Local;
addr2 = addr2Local;
// Deploy UpdateWeightRunner contract
vm.startPrank(owner);
updateWeightRunner = new MockUpdateWeightRunner(owner, addr2, false);
vm.stopPrank();
// Deploy Mock Rule and Pool
​
mockPool = new MockQuantAMMBasePool(1800, address(updateWeightRunner));
}
​
function testInitialisePoolLastRunTimeAdminAndOwnerBothPermedAdminFail() public {
uint40 blockTime = uint40(block.timestamp);
​
mockPool.setPoolRegistry(24);
vm.startPrank(owner);
updateWeightRunner.setApprovedActionsForPool(address(mockPool), 24);
vm.stopPrank();
​
vm.startPrank(admin);
vm.expectRevert("ONLYMANAGER");
updateWeightRunner.InitialisePoolLastRunTime(address(mockPool), blockTime);
}
}

Recommendations

Update the conditional logic to handle cases where both admin and owner are permitted:

Fixed Code

if (poolRegistryEntry & MASK_POOL_OWNER_UPDATES > 0 && poolRegistryEntry & MASK_POOL_QUANTAMM_ADMIN_UPDATES > 0) {
require(
msg.sender == poolRuleSettings[_poolAddress].poolManager || msg.sender == quantammAdmin,
"ONLYMANAGERORADMIN"
);
} else 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 last run time");
}
Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

invalid_manual_functions_cannot_be_called_by_admin_when_pool_owner_can

Design choice confirmed by the sponsor.

Support

FAQs

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