GroupBase Manual Review Findings

GBE-01M: Inexistent Validation of Group Entries

Type	Severity	Location
Input Sanitization		GroupBase.sol:L145

Description:

The _offerIds is not validated as a non-empty array of offers.

Impact:

Empty groups could cause certain code paths to fail instead of being executed properly.

Example:

Copy
141function addOffersToGroupInternal(uint256 _groupId, uint256[] memory _offerIds) internal {
142    // check if group can be updated
143    (uint256 sellerId, Group storage group) = preUpdateChecks(_groupId, _offerIds);
144
145    for (uint256 i = 0; i < _offerIds.length; i++) {

Recommendation:

We advise such validation to be imposed as otherwise empty groups may be created that can lead to unintended consequences.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

After discussion with the Boson Protocol team, we assessed that the check recommended is already applied indirectly by the preUpdateChecks function invoked thus nullifying this exhibit.

GBE-02M: Insufficient Evaluation of maxOffersPerGroup

Type	Severity	Location
Input Sanitization		GroupBase.sol:L196

Description:

The maxOffersPerGroup limitation can be bypassed as the preUpdateChecks function does not take into account existing checks within the group thus permitting the extension of a group's offers indefinitely.

Impact:

The maxOffersPerGroup limitation is not properly upheld by the contract which can cause denial-of-service attacks on functions utilizing groups.

Example:

Copy
171/**
172 * @dev Before performing an update, make sure update can be done
173 * and return seller id and group storage pointer for further use
174 *
175 * Reverts if:
176 *
177 * - caller is not the seller
178 * - offer ids is an empty list
179 * - number of offers exceeds maximum allowed number per group
180 * - group does not exist
181 *
182 * @param _groupId  - the id of the group to be updated
183 * @param _offerIds - array of offer ids to be removed to the group
184 * @return sellerId  - the seller Id
185 * @return group - the group details
186 */
187function preUpdateChecks(uint256 _groupId, uint256[] memory _offerIds)
188    internal
189    view
190    returns (uint256 sellerId, Group storage group)
191{
192    // make sure that at least something will be updated
193    require(_offerIds.length != 0, NOTHING_UPDATED);
194
195    // limit maximum number of offers to avoid running into block gas limit in a loop
196    require(_offerIds.length <= protocolLimits().maxOffersPerGroup, TOO_MANY_OFFERS);

Recommendation:

We advise the preUpdateChecks function to also sum the total number of offers already existing in the group with the _offerIds.length value to properly enforce the maxOffersPerGroup limitation.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

The maxOffersPerGroup limitation is now properly applied at the addOffersToGroupInternal level thus alleviating this exhibit in full.