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

Lack of Balance Validation in `withdrawFees` Function

vicent00

Lack of Balance Validation in withdrawFees Function

Description

The withdrawFees function attempts to transfer totalFees amount without checking if the contract has sufficient USDC balance. This can lead to failed transactions when the tracked fees don't match the actual contract balance.

// Root cause in the codebase with @> marks to highlight the relevant section
function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) {
revert InvalidAmount();
}
if (_to == address(0)) {
revert InvalidAddress();
}
​
// @> PROBLEM: No validation of actual contract balance before transfer
iUSDC.safeTransfer(_to, totalFees);
// @> State updated AFTER transfer, but no balance validation
totalFees = 0;
​
emit FeesWithdrawn(_to);
}

Risk

Likelihood:

  • Medium - Discrepancies between tracked fees and actual balance can occur due to failed transactions or external transfers

  • Low - The function is only called by the owner when fees are accumulated

Impact:

  • Transaction failures when attempting to withdraw more than the contract actually holds

  • Potential loss of fees if the contract balance is less than tracked totalFees

Proof of Concept

Explanation: This PoC demonstrates how the lack of balance validation can cause transaction failures when totalFees doesn't match the actual contract balance.

// Test demonstrating balance validation vulnerability
function test_withdrawFees_balance_validation() public {
// Setup: Create orders and fill them to generate fees
vm.startPrank(alice);
wbtc.approve(address(book), 2e8);
uint256 aliceId = book.createSellOrder(address(wbtc), 2e8, 180_000e6, 2 days);
vm.stopPrank();
​
vm.startPrank(dan);
usdc.approve(address(book), 200_000e6);
book.buyOrder(aliceId);
vm.stopPrank();
​
// Simulate scenario where actual balance is less than tracked fees
uint256 trackedFees = book.totalFees();
uint256 actualBalance = usdc.balanceOf(address(book));
vm.prank(owner);
usdc.transfer(address(0xdead), actualBalance - 1000e6); // Drain most balance
// Attempt to withdraw fees - this will fail
vm.startPrank(owner);
book.withdrawFees(owner); // Transaction fails, totalFees remains unchanged
vm.stopPrank();
}

Recommended Mitigation

Explanation: The fix adds balance validation before transfer to ensure the contract has sufficient USDC and prevent failed transactions.

function withdrawFees(address _to) external onlyOwner {
if (totalFees == 0) {
revert InvalidAmount();
}
if (_to == address(0)) {
revert InvalidAddress();
}
​
- iUSDC.safeTransfer(_to, totalFees);
- totalFees = 0;
​
+ // Add balance validation before transfer
+ uint256 actualBalance = iUSDC.balanceOf(address(this));
+ uint256 amountToWithdraw = totalFees > actualBalance ? actualBalance : totalFees;
+
+ if (amountToWithdraw == 0) {
+ revert InvalidAmount();
+ }
+
+ iUSDC.safeTransfer(_to, amountToWithdraw);
+ totalFees -= amountToWithdraw;
​
emit FeesWithdrawn(_to);
}
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.