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

Redundant Allowance Parameter in setAllowedSellToken

ryzen_xp

Root + Impact

Description

  • The contract maintains a mapping of allowed sell tokens. Tokens must be explicitly permitted by the owner before they can be used.

  • The _isAllowed parameter is redundant, as Solidity mappings default to false for unset keys. Thus, there's no need to explicitly set a token to false to disallow it.

function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken(); // @> _isAllowed parameter not needed
allowedSellToken[_token] = _isAllowed; // @> Mapping defaults to false, so explicit false setting is unnecessary
​
emit TokenAllowed(_token, _isAllowed);
}

Reference Files

File Function Lines Note
./src/OrderBook.sol getOrderDetailsString L273–L280

Risk

Likelihood:

  • This occurs whenever the owner attempts to disallow a token that is already unset (implicitly false).

  • It can also occur during routine maintenance when updating the allow list.

Impact:

  • Increased Gas Costs: Unnecessary storage writes (setting to false when already false) waste gas (~20,000 gas per write).

  • Code Clarity: Creates potential confusion about the need to explicitly disallow tokens.

  • Redundant Logic: Adds complexity and storage operations that are not functionally needed.

Proof of Concept (PoC)

// Current usage (wastes gas):
setAllowedSellToken(tokenA, false); // Unnecessary
​
// Suggested usage (efficient):
allowSellToken(tokenA); // Only allow when needed
// tokenB remains disallowed by default

Recommended Mitigation

  1. Simplify the Function Signature
    Remove the _isAllowed parameter and only handle explicit allowances:

function allowSellToken(address _token) external onlyOwner {
if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken();
allowedSellToken[_token] = true;
emit TokenAllowed(_token, true);
}

  1. Handle Disallowing (Optional)
    To disallow a token explicitly (if needed), either:

    • Create a separate disallowSellToken function, or

    • Rely on the default behavior where unset tokens are already considered disallowed.

  2. Update Usage
    Refactor all internal and external usages to adopt the new allowSellToken pattern.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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