Submission Details
Severity:
[L-1] Order Storage Never Cleaned Up - Potential for ID Reuse
Root + Impact
Description
-
Describe the normal behavior in one or more sentences
-
Explain the specific issue or problem in one or more sentences
## Summary
The OrderBook contract never cleans up order storage, allowing the `orders` mapping
to grow indefinitely. While order IDs are never actually reused in the current implementation,
this creates storage bloat and could theoretically lead to ID reuse issues in upgradeable contracts.
## Vulnerability Details
The contract uses a simple incrementing ID system:
mapping(uint256 => Order) public orders;
uint256 private _nextOrderId;
// In createSellOrder()
uint256 orderId = _nextOrderId++; // Always increments
orders[orderId] = Order({...});
--Issues:
- Orders are never deleted from storage
- Mapping grows indefinitely over time
- No cleanup mechanism exists
- Could cause ID reuse in upgradeable contracts
## Impact
- Storage bloat: Contract storage grows without limit
- Increased gas costs: Reading from large mappings becomes expensive
- Theoretical ID reuse: Could cause issues in upgradeable contracts
- Poor scalability: Contract becomes less efficient over time
## Affected Functions
- All functions that read from `orders` mapping
- Storage efficiency across entire contract
Risk
Likelihood:
-
Reason 1 // Describe WHEN this will occur (avoid using "if" statements)
-
Reason 2
Impact:
-
Impact 1
-
Impact 2
Proof of Concept
# Proof of Concept
## Simple Explanation
1. User creates order #1
2. Order #1 gets filled/cancelled
3. Order #1 data remains in storage forever
4. User creates order #2
5. Storage keeps growing with no cleanup
## The Problem
Orders are never deleted from storage:
// Lines 123-131: createSellOrder()
orders[orderId] = Order({
id: orderId,
seller: msg.sender,
// ... order data stored
});
// No cleanup when order is filled/cancelled
// Storage slot remains occupied forever
## What Happens
- Orders accumulate in storage indefinitely
- No deletio* when orders are filled/cancelled
- Mapping grows without limit
- Gas costs increase over time
- Potential ID reuse in upgradeable contracts
Recommended Mitigation
# Mitigation
Orders are never deleted from storage, causing the `orders` mapping to grow indefinitely
and potentially leading to ID reuse issues.
Vulnerable Code:
// Orders are stored but never cleaned up
orders[orderId] = Order({...});
// When order is filled/cancelled, data remains in storage
order.isActive = false; // Only flag is changed, data not deleted
--The Problem:
Storage grows without limit, increasing gas costs and creating potential for ID
reuse in upgradeable contracts.
## How to Fix
### Solution 1: Delete Orders After Completion
function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// ... validation and transfer logic ...
// Delete order from storage
delete orders[_orderId]; // Clean up storage
}
function cancelSellOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// ... validation and transfer logic ...
// Delete order from storage
delete orders[_orderId]; // Clean up storage
}
Updates
Lead Judging Commences
yeahchibyke 7 days ago
Submission Judgement Published Assigned finding tags:
Expired orders can cause backlog
By design only `seller` can call `cancelSellOrder()` on their `order`. But when an `order` expires, and the `seller` doesn't have access to the protocol, the expired `order `should be be able to be cancelled by an `admin`.
Rewards breakdown
nSLOC
217This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.