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

Centralized admin control in UpdateWeightRunner contract

zubyoz

Summary

The UpdateWeightRunner contract implements a centralized administrative control pattern where a single admin address has unrestricted and immediate control over critical protocol parameters, including fees, oracles, and pool permissions. This design creates a significant security risk as compromise of the admin key or malicious admin actions could lead to immediate protocol manipulation and potential loss of user funds.

Vulnerability Details

Affected code sections:

  1. Centralized Admin Control:
    https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L104-L110

address public immutable quantammAdmin;
​
constructor(address _quantammAdmin, address _ethOracle) Ownable(msg.sender) {
require(_quantammAdmin != address(0), "Admin cannot be default address");
quantammAdmin = _quantammAdmin;
}

  1. Fee Management:
    https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L126-L133

function setQuantAMMSwapFeeTake(uint256 _quantAMMSwapFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
quantAMMSwapFeeTake = _quantAMMSwapFeeTake;
emit SwapFeeTakeSet(oldSwapFee, _quantAMMSwapFeeTake);
}

  1. Oracle Control:
    https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L204-L215

function addOracle(OracleWrapper _oracle) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
approvedOracles[address(_oracle)] = true;
emit OracleAdded(address(_oracle));
}
​
function setETHUSDOracle(address _ethUsdOracle) public {
require(msg.sender == quantammAdmin, "ONLYADMIN");
ethOracle = OracleWrapper(_ethUsdOracle);
}

  1. Pool Management:
    https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/UpdateWeightRunner.sol#L227-L231

function setApprovedActionsForPool(address _pool, uint256 _actions) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
approvedPoolActions[_pool] = _actions;
}

The vulnerability occurs through:

  1. Admin key compromise:

    • Attacker gains control of admin private key

    • Can immediately execute any admin function

    • No delay or additional verification required

  2. Malicious admin actions:

    • Admin can set arbitrary fees up to 100%

    • Can add compromised oracles

    • Can disable critical pool functions

    • Changes take effect immediately

  3. No safety mechanisms:

    • No timelock delays

    • No multi-signature requirements

    • No parameter bounds

    • No emergency pause functionality

Impact

The vulnerability has severe implications:

  1. Financial Impact:

    • Immediate manipulation of fees up to 100%

    • Potential loss of user funds through excessive fees

    • Price manipulation through compromised oracles

  2. Protocol Security:

    • Complete control over pool operations

    • Ability to add malicious oracles

    • Disable critical protocol functions

    • No recovery mechanism

  3. Trust and Reputation:

    • Single point of failure risks

    • Potential permanent damage to protocol reputation

    • Loss of user confidence in protocol security

Tools Used

Manual code review

Recommendations

  1. Implement Timelock Mechanism:

contract UpdateWeightRunner is TimelockController {
uint256 public constant DELAY = 2 days;
function scheduleParameterChange(bytes32 paramId, uint256 newValue) external {
require(msg.sender == quantammAdmin);
_schedule(paramId, newValue, DELAY);
}
function executeChange(bytes32 changeId) external {
require(block.timestamp >= pendingChanges[changeId]);
// Execute change
}
}

  1. Add Multi-signature Requirements:

contract UpdateWeightRunner {
uint256 public constant REQUIRED_SIGNATURES = 3;
mapping(bytes32 => uint256) public signatureCount;
function proposeChange(bytes32 changeId) external {
require(isAdmin[msg.sender]);
signatureCount[changeId]++;
if(signatureCount[changeId] >= REQUIRED_SIGNATURES) {
executeChange(changeId);
}
}
}

  1. Implement Parameter Bounds:

contract UpdateWeightRunner {
uint256 public constant MAX_FEE = 0.1e18; // 10%
function setFee(uint256 newFee) external {
require(newFee <= MAX_FEE, "Fee exceeds maximum");
// Set fee
}
}

  1. Add Emergency Controls:

contract UpdateWeightRunner {
bool public emergency;
modifier notEmergency() {
require(!emergency, "System is paused");
_;
}
function triggerEmergency() external {
require(hasRole(EMERGENCY_ROLE, msg.sender));
emergency = true;
}
}

  1. Implement Role-Based Access Control:

contract UpdateWeightRunner is AccessControl {
bytes32 public constant FEE_ADMIN = keccak256("FEE_ADMIN");
bytes32 public constant ORACLE_ADMIN = keccak256("ORACLE_ADMIN");
function setFee(uint256 newFee) external {
require(hasRole(FEE_ADMIN, msg.sender));
// Set fee
}
}

Here are few report discussing the risks of centralized admin control

https://solodit.cyfrin.io/issues/centralization-risks-with-a-lot-of-power-vested-in-the-reporter-role-cyfrin-none-casimir-markdown
https://solodit.cyfrin.io/issues/centralization-can-lead-to-unfair-changes-without-users-notice-zokyo-none-zap-markdown

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
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.