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

[M-02] Incorrect duration update logic in OrderBook::amendSellOrder() can cause orders to be able to remain active for more than intended maximum duration

soarinskysagar

Root + Impact

Description

  • deadlineTimestamp variable is used to keep track of the maximun timestamp till which the order will be active and is being set in createSellOrder(). the maximum duration limit for an order is 3 days.

  • The following logic to update deadlineTimestamp variable can cause the order to go on for more than the intended limit of 3 days, given that the function is called with a new duration before order expiry. This calculates the new duration from the moment the amendSellOrder() is called, instead of the moment when the order was created.

function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
) public {
Order storage order = orders[_orderId];
​
// Validation checks
if (order.seller == address(0)) revert OrderNotFound(); // Check if order exists
if (order.seller != msg.sender) revert NotOrderSeller();
if (!order.isActive) revert OrderAlreadyInactive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // Cannot amend expired order
if (_newAmountToSell == 0) revert InvalidAmount();
if (_newPriceInUSDC == 0) revert InvalidPrice();
if (_newDeadlineDuration == 0 || _newDeadlineDuration > MAX_DEADLINE_DURATION) revert InvalidDeadline();
​
@> uint256 newDeadlineTimestamp = block.timestamp + _newDeadlineDuration;
IERC20 token = IERC20(order.tokenToSell);

Risk

Likelihood:

  • Whenever amendSellOrder() function is called on an active order with a new timestamp

Impact:

  • The order always gets updated with a new duration timestamp, the duration being calculated from the moment the amendSellOrder() is called

Proof of Concept

Place this function into TestOrderBook.t.sol and run with forge test --mt test_extendDuration. This passes, which means that the function is indeed active even after the maximum duration limit of 3 days.

function test_extendDuration() public {
// alice creates sell order for wbtc
vm.startPrank(alice);
wbtc.approve(address(book), 2e8);
uint256 aliceId = book.createSellOrder(address(wbtc), 2e8, 180_000e6, 3 days);
assert(aliceId == 1);
​
// fast forward to just 1 hour before expiry
vm.warp(block.timestamp + 2 days + 23 hours);
​
book.amendSellOrder(aliceId, 2e8, 180_000e6, 3 days);
​
// 4 days and 23 hours after order creation
vm.warp(block.timestamp + 2 days);
​
OrderBook.Order memory order = book.getOrder(aliceId);
​
assertGt(order.deadlineTimestamp, block.timestamp);
}

Recommended Mitigation

Add a new field in Order struct which keeps track of the timestamp of creation. Then update the new deadline timestamp from the creation timestamp in amendSellOrder()

struct Order {
uint256 id;
address seller;
address tokenToSell; // Address of wETH, wBTC, or wSOL
uint256 amountToSell; // Amount of tokenToSell
uint256 priceInUSDC; // Total USDC price for the entire amountToSell
uint256 deadlineTimestamp; // Block timestamp after which the order expires
bool isActive; // Flag indicating if the order is available to be bought
+ uint256 creationTimestamp;
}
.
.
.
function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
if (!allowedSellToken[_tokenToSell]) revert InvalidToken();
if (_amountToSell == 0) revert InvalidAmount();
if (_priceInUSDC == 0) revert InvalidPrice();
if (_deadlineDuration == 0 || _deadlineDuration > MAX_DEADLINE_DURATION) revert InvalidDeadline();
​
uint256 deadlineTimestamp = block.timestamp + _deadlineDuration;
+ uint256 creationTimestamp = block.timestamp;
uint256 orderId = _nextOrderId++;
​
IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
​
// Store the order
orders[orderId] = Order({
id: orderId,
seller: msg.sender,
tokenToSell: _tokenToSell,
amountToSell: _amountToSell,
priceInUSDC: _priceInUSDC,
deadlineTimestamp: deadlineTimestamp,
- isActive: true
+ isActive: true,
+ creationTimestamp = creationTimestamp
});
​
emit OrderCreated(orderId, msg.sender, _tokenToSell, _amountToSell, _priceInUSDC, deadlineTimestamp);
return orderId;
}
.
.
.
- uint256 newDeadlineTimestamp = block.timestamp + _newDeadlineDuration;
+ uint256 newDeadlineTimestamp = order.creationTimestamp + _newDeadlineDuration;
+ assert(newDeadlineTimestamp > block.timestamp);
Updates

Lead Judging Commences

yeahchibyke Lead Judge
about 1 month ago
yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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