OrderBook
First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Centralization Risk / Owner Malicious Withdrawal

nagatejakachapuram

Description


The emergencyWithdrawERC20 function allows the contract owner to withdraw any ERC20 token held by the contract, excluding the four specified core trading tokens. This creates a significant centralization risk, enabling a malicious or compromised owner to drain non-core funds.


Risk


Root Cause: The emergencyWithdrawERC20 function, accessible only by the owner, directly transfers arbitrary ERC20 tokens.

Solidity

function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
// ... (Skipping checks for core tokens and address(0))
IERC20 token = IERC20(_tokenAddress);
@> token.safeTransfer(_to, _amount); // Owner can transfer any non-core ERC20
// ...
}
​

Likelihood: High. The function is an explicit capability of the owner. Its existence is the risk.

Impact: High.

  • Loss of Funds: Users' accidentally sent tokens (non-core) can be permanently lost if the owner withdraws them.

  • Rug Pull Vector: While core tokens are protected, other potentially valuable assets could be drained, leading to a trust breakdown and financial loss for users.

  • Centralization: Contradicts decentralized principles by giving a single entity arbitrary withdrawal power.


Proof of Concept

The owner simply calls the function to withdraw unintended or accidentally sent tokens.

Scenario:

  1. A user accidentally sends 1000 DAI (a non-core ERC20) to the OrderBook contract.

  2. The OrderBook contract now holds 1000 DAI.

  3. The contract owner calls emergencyWithdrawERC20(address(DAI), 1000, owner_address).

  4. The 1000 DAI are transferred from the OrderBook to the owner, without user consent or possibility of recovery.


Recommended Mitigation


Option 1 (Strongest): Remove the Function: Eliminate this function to remove the rug-pull vector and uphold decentralization. Accidental token transfers would be permanently locked unless future upgrades (with robust governance) allow recovery.

Diff

- function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
- if (
- _tokenAddress == address(iWETH) || _tokenAddress == address(iWBTC) || _tokenAddress == address(iWSOL)
- || _tokenAddress == address(iUSDC)
- ) {
- revert("Cannot withdraw core order book tokens via emergency function");
- }
- if (_to == address(0)) {
- revert InvalidAddress();
- }
- IERC20 token = IERC20(_tokenAddress);
- token.safeTransfer(_to, _amount);
-
- emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
- }
​

Option 2 (Alternative): Implement Decentralized Control: If emergency withdrawal is deemed essential, transfer control to a multi-signature wallet or DAO governance. Additionally, implement a time-lock (e.g., 7 days) on withdrawals to allow community reaction.

Reference Files:

  • src/OrderBook.sol

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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