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

Redundant Status Logic Causes Inconsistent Order Status Reporting

mukulkolpe

Inconsistent Status Reporting in getOrderDetailsString

Description

  • The getOrderDetailsString function is designed to provide accurate order status information to users and front-end applications

  • The function contains redundant and conflicting status assignment logic that overwrites initial status determinations, leading to inconsistent and confusing status messages

function getOrderDetailsString(uint256 _orderId) public view returns (string memory details) {
Order storage order = orders[_orderId];
if (order.seller == address(0)) revert OrderNotFound();
​
// ... token symbol logic ...
​
// @> Initial status assignment
string memory status = order.isActive
? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")
: "Inactive (Filled/Cancelled)";
// @> Redundant status logic that overwrites the initial assignment
if (order.isActive && block.timestamp >= order.deadlineTimestamp) {
status = "Expired (Awaiting Cancellation)";
} else if (!order.isActive) {
status = "Inactive (Filled/Cancelled)";
} else {
status = "Active";
}
​
// ... rest of function uses the final status value
}

Risk

Likelihood: High

  • Every call to getOrderDetailsString executes this redundant logic

  • The function is likely called frequently by front-end applications and users

  • The logic flow is guaranteed to produce the overwritten result

Impact: Low

  • Users and applications receive inconsistent status information

  • Debugging and maintenance become more difficult due to redundant code

  • Potential confusion about actual order states

  • Code maintainability issues but no direct financial impact

Recommended Mitigation

function getOrderDetailsString(uint256 _orderId) public view returns (string memory details) {
Order storage order = orders[_orderId];
if (order.seller == address(0)) revert OrderNotFound();
​
// ... token symbol logic ...
​
- string memory status = order.isActive
- ? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")
- : "Inactive (Filled/Cancelled)";
- if (order.isActive && block.timestamp >= order.deadlineTimestamp) {
- status = "Expired (Awaiting Cancellation)";
- } else if (!order.isActive) {
- status = "Inactive (Filled/Cancelled)";
- } else {
- status = "Active";
- }
​
+ string memory status;
+ if (!order.isActive) {
+ status = "Inactive (Filled/Cancelled)";
+ } else if (block.timestamp >= order.deadlineTimestamp) {
+ status = "Expired (Awaiting Cancellation)";
+ } else {
+ status = "Active";
+ }
​
details = string(
abi.encodePacked(
"Order ID: ",
order.id.toString(),
"\n",
"Seller: ",
Strings.toHexString(uint160(order.seller), 20),
"\n",
"Selling: ",
order.amountToSell.toString(),
" ",
tokenSymbol,
"\n",
"Asking Price: ",
order.priceInUSDC.toString(),
" USDC\n",
"Deadline Timestamp: ",
order.deadlineTimestamp.toString(),
"\n",
"Status: ",
status
)
);
​
return details;
}

Benefits of Mitigation:

  1. Eliminates redundancy - Single, clear status determination logic

  2. Improves readability - Easier to understand and maintain

  3. Reduces gas costs - Fewer unnecessary operations

  4. Consistent behavior - Same status results with cleaner code

  5. Better maintainability - Future status changes only need to be made in one place

Updates

Lead Judging Commences

yeahchibyke Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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