OrderBook

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

Overpowered Ownership Model Breaks Trust and Decentralization Guarantees

Overpowered Ownership Model Breaks Trust and Decentralization Guarantees

Description

  • In normal behavior, the contract is expected to act as a decentralized order book, allowing trustless peer-to-peer token exchange with minimal central intervention.

  • However, the contract's logic gives the single owner the authority to unilaterally change key protocol parameters, withdraw all collected fees, and execute emergency withdrawals. This undermines decentralization and opens the door to rug-pull scenarios or abuse of power.

@> function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
allowedSellToken[_token] = _isAllowed;
}
@> function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
token.safeTransfer(_to, _amount);
}
@> function withdrawFees(address _to) external onlyOwner {
iUSDC.safeTransfer(_to, totalFees);
}

Risk

Likelihood:

  • This issue is always present due to the absence of governance or multi-signature restrictions.

  • It will occur as soon as a malicious or compromised owner calls any of the admin-level withdrawal functions.

Impact:

  • A malicious owner can drain protocol fees or tokens via withdrawFees() or emergencyWithdrawERC20().

  • Trust in the protocol's integrity is compromised, making it unsuitable for production DeFi environments where decentralization is expected.

Proof of Concept

// Assume the owner is malicious or becomes compromised
// The owner can rug all fees:
orderBook.withdrawFees(maliciousEOA);
// The owner can allow a malicious token for trading:
orderBook.setAllowedSellToken(maliciousToken, true);
// The owner can drain tokens not explicitly blocked:
orderBook.emergencyWithdrawERC20(anyERC20, balance, attacker);

Recommended Mitigation

  • Implement a DAO or governance-based control mechanism instead of a single-owner model.

- function withdrawFees(address _to) external onlyOwner {
+ function withdrawFees(address _to) external onlyOwner {
+ require(msg.sender == address(governanceMultisig), "Only multisig");
require(totalFees > 0, "No fees");
require(_to != address(0), "Invalid address");
iUSDC.safeTransfer(_to, totalFees);
totalFees = 0;
}
- function emergencyWithdrawERC20(...) external onlyOwner {
+ function emergencyWithdrawERC20(...) external onlyOwner {
+ require(msg.sender == address(governanceMultisig), "Only multisig");
...
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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