Omniscia Boson Protocol Audit

PauseHandlerFacet Manual Review Findings

PauseHandlerFacet Manual Review Findings

PHF-01M: Incorrect Revert Condition Specification

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:

contracts/protocol/facets/PauseHandlerFacet.sol
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 once
110 *
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 reference
116 ProtocolLib.ProtocolStatus storage status = protocolStatus();
117
118 // Toggle all regions if none are specified.
119 if (_regions.length == 0) {
120 // Store the toggle scenario
121 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 regions
129 // Use "or" to get the correct value even if the same region is specified more than once
130 for (uint256 i = 0; i < _regions.length; i++) {
131 // Get enum value as power of 2
132 region = 1 << uint256(_regions[i]);
133 incomingScenario |= region;
134 }
135
136 // Store the toggle scenario
137 if (_pause) {
138 // for pausing, just "or" the incoming scenario with the existing one
139 // equivalent to summation
140 status.pauseScenario |= incomingScenario;
141 } else {
142 // for unpausing, "and" the inverse of the incoming scenario with the existing one
143 // equivalent to subtraction
144 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.