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

Critical Absence of Circuit Breaker Pattern Makes Contract Vulnerable to Exploits Without Mitigation Options

0xstn_

Root + Impact

Description

Normal Behavior

The OrderBook contract allows trading between tokens and USDC with order creation, cancellation, and buying functionality. While there is an emergency withdrawal function for non-core tokens, the contract lacks a crucial security feature found in modern DeFi protocols.

Specific Issue

The contract does not implement a circuit breaker (pause) mechanism, which is an essential security feature for DeFi protocols. In case a vulnerability is discovered or exploited, there is no way to temporarily pause the contract's core functionality while a fix is being prepared.

// No circuit breaker functionality exists in the contract
contract OrderBook is Ownable {
// Missing pause state variables
// Missing pause modifiers
// Missing pause/unpause functions
​
// Core functions lack pause checks
function createSellOrder(...) public returns (uint256) { ... }
function amendSellOrder(...) public { ... }
function cancelSellOrder(uint256 _orderId) public { ... }
function buyOrder(uint256 _orderId) public { ... }
// ...
}
​

While the contract does include an emergency withdrawal function, this only applies to non-core tokens:

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");
}
// ...
}
​

This means that if a critical vulnerability is discovered in any of the main contract functions, there is no way to prevent users from continuing to interact with the vulnerable functions, potentially leading to significant loss of funds

Here is some references :-

Risk

Likelihood:

  1. Given the complexity of the contract and its interaction with external tokens, the chance of a vulnerability being discovered after deployment is significant.

  2. If a zero-day vulnerability is discovered by a malicious actor, there's no way to prevent exploitation while a fix is developed.

  3. Many modern attacks against DeFi protocols involve multiple transactions that could be prevented if admins could pause the contract upon detecting suspicious activity.

Impact:

  1. In case of a discovered vulnerability, users' funds would remain at risk as core contract functionality cannot be paused.

  2. The absence of a circuit breaker could result in complete draining of the contract if a critical vulnerability is found.

  3. If an economic attack (e.g., price manipulation) occurs, there's no way to temporarily halt operations until market conditions normalize.

  4. The contract would need to be completely redeployed to address any discovered issues, causing significant disruption.

Proof of Concept

i have senario to tell , lets imaginne

  1. A vulnerability is discovered in the buyOrder function that allows an attacker to purchase orders without paying the correct amount of USDC.

  2. The protocol team becomes aware of the vulnerability but cannot pause the contract.

  3. While they prepare a fix and attempt to deploy a new contract, the attacker exploits the vulnerability multiple times.

  4. Without a pause mechanism, all funds in the contract are at risk until a complete migration to a new contract is performed.

    that what makes it too danger ...

// Scenario: Vulnerability discovered in buyOrder function
​
// Attacker exploits the vulnerability
function exploit(uint256 orderId) external {
// Exploit the vulnerability
vulnerableOrderBook.buyOrder(orderId); // Vulnerable call
​
// Drain funds
// ...
}
​
// Contract owner has no way to prevent further exploitation
// No pause function exists to call:
// orderBook.pause(); // This function does not exist
​

Recommended Mitigation

+ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+ import {Pausable} from "@openzeppelin/contracts/security/Pausable.sol";
​
- contract OrderBook is Ownable {
+ contract OrderBook is Ownable, Pausable {
// ...existing code...
​
// Add pause/unpause functions
+ function pause() external onlyOwner {
+ _pause();
+ }
+
+ function unpause() external onlyOwner {
+ _unpause();
+ }
​
// Add whenNotPaused modifiers to all state-changing functions
- function createSellOrder(
+ function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
- ) public returns (uint256) {
+ ) public whenNotPaused returns (uint256) {
// ...existing code...
}
​
- function amendSellOrder(
+ function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
- ) public {
+ ) public whenNotPaused {
// ...existing code...
}
​
- function cancelSellOrder(uint256 _orderId) public {
+ function cancelSellOrder(uint256 _orderId) public whenNotPaused {
// ...existing code...
}
​
- function buyOrder(uint256 _orderId) public {
+ function buyOrder(uint256 _orderId) public whenNotPaused {
// ...existing code...
}
​
// Consider adding an emergency function to allow order cancellation even when paused
+ function emergencyCancelOrder(uint256 _orderId) external onlyOwner {
+ Order storage order = orders[_orderId];
+ if (order.seller == address(0)) revert OrderNotFound();
+ if (!order.isActive) revert OrderAlreadyInactive();
+
+ order.isActive = false;
+ IERC20(order.tokenToSell).safeTransfer(order.seller, order.amountToSell);
+
+ emit OrderCancelled(_orderId, order.seller);
+ }
​
// Also add a comprehensive emergency withdrawal function for core tokens
+ function emergencyWithdrawCoreTokens(address _tokenAddress, uint256 _amount, address _to) external onlyOwner whenPaused {
+ if (_to == address(0)) revert InvalidAddress();
+ IERC20(_tokenAddress).safeTransfer(_to, _amount);
+ emit EmergencyWithdrawal(_tokenAddress, _amount, _to);
+ }
}
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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