Omniscia Boson Protocol Audit
PauseHandlerFacet Manual Review Findings
PauseHandlerFacet Manual Review Findings
PHF-01M: Incorrect Revert Condition Specification
| Type | Severity | Location |
|---|---|---|
| Code Style | ![]() | PauseHandlerFacet.sol:L33, L52, L109 |
Description:
The referenced comments insinuate that the PauseHandlerFacet::pause, PauseHandlerFacet::unpause, and PauseHandlerFacet::togglePause functions will revert if a duplicate region is specified, however, the code itself makes sure to calculate the correct state value even if duplicate regions exist.
Impact:
Callers of the PauseHandlerFacet functions may assume that their regions are guaranteed to be unique when that is not the case.
Example:
103/**104 * @notice Toggles pause/unpause for some or all of the protocol.105 *106 * Toggle all regions if none are specified.107 *108 * Reverts if:109 * - A region is specified more than once110 *111 * @param _regions - an array of regions to pause/unpause. See: {BosonTypes.PausableRegion}112 * @param _pause - a boolean indicating whether to pause (true) or unpause (false)113 */114function togglePause(BosonTypes.PausableRegion[] calldata _regions, bool _pause) internal {115 // Cache protocol status for reference116 ProtocolLib.ProtocolStatus storage status = protocolStatus();117
118 // Toggle all regions if none are specified.119 if (_regions.length == 0) {120 // Store the toggle scenario121 status.pauseScenario = _pause ? ALL_REGIONS_MASK : 0;122 return;123 }124
125 uint256 region;126 uint256 incomingScenario;127
128 // Calculate the incoming scenario as the sum of individual regions129 // Use "or" to get the correct value even if the same region is specified more than once130 for (uint256 i = 0; i < _regions.length; i++) {131 // Get enum value as power of 2132 region = 1 << uint256(_regions[i]);133 incomingScenario |= region;134 }135
136 // Store the toggle scenario137 if (_pause) {138 // for pausing, just "or" the incoming scenario with the existing one139 // equivalent to summation140 status.pauseScenario |= incomingScenario;141 } else {142 // for unpausing, "and" the inverse of the incoming scenario with the existing one143 // equivalent to subtraction144 status.pauseScenario &= ~incomingScenario;145 }146}Recommendation:
We advise the code to either prevent duplicate regions or remove the revert condition from the documentation, either of which we consider an adequate resolution and the latter of which we assume to be appropriate action based on the PauseHandlerFacet::togglePause function's implementation.
Alleviation (2b9f60b6c3323fd234b570089ceff924cdb5851c):
The documentation was corrected by the Boson Protocol team rendering this exhibit alleviated.
