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

Centralization Risk + Single Point of Failure

blee

Description

  • A decentralized trading protocol should have distributed control to prevent single points of failure and abuse of power.

  • The contract owner has excessive control over critical protocol functions including fee withdrawal, token allowlist management, and emergency withdrawals, creating centralization risks.

function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken();
allowedSellToken[_token] = _isAllowed;
emit TokenAllowed(_token, _isAllowed);
}
​
function emergencyWithdrawERC20(address _tokenAddress, uint256 _amount, address _to) external onlyOwner {
// Can withdraw any non-core tokens
IERC20 token = IERC20(_tokenAddress);
token.safeTransfer(_to, _amount);
emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
}
​
function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) revert InvalidAmount();
if (_to == address(0)) revert InvalidAddress();
iUSDC.safeTransfer(_to, totalFees);
totalFees = 0;
emit FeesWithdrawn(_to);
}

Risk

Likelihood:

  • When the owner's private key is compromised or stolen

  • When the owner acts maliciously or makes poor decisions

  • When the owner becomes unavailable (lost keys, death, etc.)

Impact:

  • Owner can disable trading for any token by setting allowedSellToken to false

  • Owner can extract all protocol fees at any time without community approval

  • Owner can withdraw any accidentally sent tokens, potentially including user funds

  • Protocol becomes unusable if owner key is lost

Proof of Concept

Centralization Attack Scenario: This demonstrates how the owner can abuse privileges to harm users and the protocol.

function testCentralizationRisk() public {
// Protocol accumulates fees
vm.startPrank(alice);
weth.approve(address(book), 1e18);
uint256 orderId = book.createSellOrder(address(weth), 1e18, 3000e6, 1 days);
vm.stopPrank();
​
vm.startPrank(dan);
usdc.approve(address(book), 3000e6);
book.buyOrder(orderId);
vm.stopPrank();
​
assertEq(book.totalFees(), 90e6); // 3% of 3000 USDC
​
// Owner can drain all fees
vm.startPrank(owner);
book.withdrawFees(owner);
vm.stopPrank();
​
assertEq(book.totalFees(), 0);
assertEq(usdc.balanceOf(owner), 90e6);
​
// Owner can disable wETH trading
vm.startPrank(owner);
book.setAllowedSellToken(address(weth), false);
vm.stopPrank();
​
// Users can no longer create wETH orders
vm.startPrank(alice);
vm.expectRevert(OrderBook.InvalidToken.selector);
book.createSellOrder(address(weth), 1e18, 3000e6, 1 days);
vm.stopPrank();
}

Abuse Scenarios:

  1. Immediate fee extraction: Owner can withdraw all accumulated fees without community approval

  2. Market manipulation: Owner can suddenly disable popular tokens, causing market disruption

  3. Selective censorship: Owner can prevent specific users from trading by disabling their preferred tokens

  4. Emergency abuse: Owner can use emergencyWithdrawERC20 to extract legitimate user funds under false pretenses

  5. Key compromise: If owner's private key is stolen, attacker has full control over the protocol

  6. Availability risk: Lost owner keys make the protocol permanently unable to adapt or collect fees

Recommended Mitigation

+ import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
​
- contract OrderBook is Ownable {
+ contract OrderBook is Ownable {
+ uint256 public constant TIMELOCK_DELAY = 48 hours;
+ mapping(bytes32 => uint256) public pendingActions;
​
+ modifier onlyAfterTimelock(bytes32 actionHash) {
+ require(pendingActions[actionHash] != 0, "Action not scheduled");
+ require(block.timestamp >= pendingActions[actionHash], "Timelock not expired");
+ _;
+ delete pendingActions[actionHash];
+ }
​
+ function scheduleAction(bytes32 actionHash) external onlyOwner {
+ pendingActions[actionHash] = block.timestamp + TIMELOCK_DELAY;
+ }
​
- function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
+ function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner onlyAfterTimelock(keccak256(abi.encode(_token, _isAllowed))) {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken();
allowedSellToken[_token] = _isAllowed;
emit TokenAllowed(_token, _isAllowed);
}
​
- function withdrawFees(address _to) external onlyOwner {
+ function withdrawFees(address _to) external onlyOwner onlyAfterTimelock(keccak256(abi.encode(_to))) {
if (totalFees == 0) revert InvalidAmount();
if (_to == address(0)) revert InvalidAddress();
iUSDC.safeTransfer(_to, totalFees);
totalFees = 0;
emit FeesWithdrawn(_to);
}
}

Why this works:

  • Timelock mechanism: Forces a 48-hour delay before executing privileged actions

  • Community visibility: Users can see scheduled actions and exit if they disagree

  • Prevents immediate abuse: Compromised keys cannot instantly drain funds or disable tokens

  • Maintains owner control: Owner can still perform necessary actions after proper notice

Better alternatives for full decentralization:

  1. Multi-signature wallet: Require multiple signatures for critical operations

  2. DAO governance: Community voting on fee withdrawals and token changes

  3. Immutable allowlist: Pre-approve tokens at deployment, remove owner token control

  4. Automated fee distribution: Send fees to staking rewards or burn mechanisms

  5. Proxy pattern with governance: Upgradeable contract controlled by token holders

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.