Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Inadvertent revert caused by logic in require statement within modifier of SettlementBranch::_fillOrder function

gbadebosmith

Summary

The fillOrderfunction found on lines 157, 321, and 356 (the source function) will always revert due to a coflicting requirement statement built into the modifiers for onlyMarketOrderKeeperand onlyOffChainOrdersKeepers. The culprit require statement is on line 565 and is called _requireIsKeeper.

Vulnerability Details

The problem with the conflicting require statement is in the modifiers that protect the functions holding the _fillOrderis as follows.

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/perpetuals/branches/SettlementBranch.sol#L108-L167

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/perpetuals/branches/SettlementBranch.sol#L188-L329

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/perpetuals/branches/SettlementBranch.sol#L564-L569

/// @param keeper The keeper address.
function _requireIsKeeper(address sender, address keeper) internal pure {
if (sender != keeper && keeper != address(0)) {
revert Errors.OnlyKeeper(sender, keeper);
}
}

The if statement above requires that if the keeper is not the sender and if the keeper is not address zero then revert.

But the problem is that if you are a valid keeper and also if you are not a valid keeper then the function would revert either way. lol.

Impact

The impact is that no one can place or cancel orders.

Tools Used

Manual review.

Recommendations

Amend the _requireIsKeeper function as follows.

/// @param keeper The keeper address.
function _requireIsKeeper(address sender, address keeper) internal pure {
+ if (sender != keeper || keeper == address(0)) {
- if (sender != keeper && keeper != address(0)) {
revert Errors.OnlyKeeper(sender, keeper);
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[INVALID]`_requireIsKeeper` doesn't quite work well

Support

FAQs

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