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

_requireIsKeeper access control bypass - If keeper == address(0), any address may fill orders or perform settlement.

blockchomper

Summary

The _requireIsKeeper check that is implemented in the onlyMarketOrderKeeper and onlyOffChainOrdersKeeper in SettlementBranch.sol can be bypassed if the keeper address is set to 0.

​
modifier onlyMarketOrderKeeper(uint128 marketId) {
SettlementConfiguration.Data storage settlementConfiguration =
SettlementConfiguration.load(marketId, SettlementConfiguration.MARKET\_ORDER\_CONFIGURATION\_ID);
​
_requireIsKeeper(msg.sender, settlementConfiguration.keeper);
_;
}
​
modifier onlyOffchainOrdersKeeper(uint128 marketId) {
SettlementConfiguration.Data storage settlementConfiguration =
SettlementConfiguration.load(marketId, SettlementConfiguration.OFFCHAIN_ORDERS_CONFIGURATION_ID);
​
_requireIsKeeper(msg.sender, settlementConfiguration.keeper);
_;
}
​
function _requireIsKeeper(address sender, address keeper) internal pure {
if (sender != keeper && keeper != address(0)) {
revert Errors.OnlyKeeper(sender, keeper);
}
}

Vulnerability Details

This if statement in requireIsKeeper uses logical AND (&&). Both conditions must be true for the revert to happen:

  1. sender != keeper

  2. keeper != address(0)

Hence, if keeper == address(0), condition #2 is false, and the entire expression in the if statement evaluates to false regardless of sender != keeper. This means the revert is never triggered, letting anyone call the function.

Admins could forget to set or update the keeper address to 0 inadvertently making the function open to the public.

Impact

The settlement/market order calls (fillMarketOrder, fillOffchainOrders, etc.) are supposed to be restricted to a single address (the designated keeper). When keeper == address(0), the condition in _requireIsKeeper short‐circuits, causing no revert for any caller. This effectively grants public access to the function.

Tools Used

Manual Review of Access Control Modifiers

Recommendations

A simple refactor can enforce that the keeper address cannot be zero

function _requireIsKeeper(address sender, address keeper) internal pure {
if (keeper == address(0)) revert Errors.NoKeeperSet(); // new or existing revert
​
if (sender != keeper) revert Errors.OnlyKeeper(sender, keeper);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
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.