code-423n4 / 2023-07-lens-findings

0 stars 0 forks source link

SeaDropMintPublicationAction._distributeFees will revert due to division by zero #97

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/modules/act/seadrop/SeaDropMintPublicationAction.sol#L231-L232

Vulnerability details

Impact

LensHub.act is used to perform the specified action on the specified publication, which must be the action enabled by the publication when it was created. The publicationActionParams parameter of LensHub.act is the PublicationActionParams structure, which is passed by the caller. A publication enables SeaDropMintPublicationAction, and the caller can execute this action via LensHub.act. If  PublicationActionParams.referrerProfileIds set by caller is an empty array, tx will revert due to division by zero. Setting referrerProfileIds to empty is not a malicious input, but a normal business scenario.

Proof of Concept

The flow of executing SeaDropMintPublicationAction via LensHub.act is as follows::

LensHub.act //PublicationActionParams.actionModuleAddress specified by caller is equal to SeaDropMintPublicationAction
  ActionLib.act
    _isActionEnabled(_actedOnPublication, actionModuleId)   //not enabled, revert.
    ValidationLib.validateReferrersAndGetReferrersPubTypes  //publicationActionParams.referrerProfileIds can be empty array
    IPublicationActionModule(actionModuleAddress).processPublicationAction //=SeaDropMintPublicationAction.processPublicationAction

SeaDropMintPublicationAction.processPublicationAction internally transfers the wmatic required by minting nft from the caller, mints nft to actorProfileOwner, and finally distributes fees according to whether expectedFees is greater than 0. Let's look at the code snippet of _distributeFees:

File: contracts\modules\act\seadrop\SeaDropMintPublicationAction.sol
219:     function _distributeFees(
220:         uint256 feesToDistribute,
221:         uint256 mintPaymentAmount,
222:         address lensTreasuryAddress,
223:         CollectionData memory collectionData,
224:         Types.ProcessActionParams calldata processActionParams //from caller
225:     ) internal {
226:         // Wrap MATIC back into WMATIC.
227:         WMATIC.deposit{value: feesToDistribute}();
228: 
229:         uint256 referrersCut = (mintPaymentAmount * collectionData.referrersFeeBps) / MAX_BPS;
230:         
231:->       uint256 referrersQuantity = processActionParams.referrerProfileIds.length;//@audit array.length = 0 without any referrer.
232:->       uint256 feePerReferrer = referrersCut / referrersQuantity; //@audit revert here due to division by 0

As mentioned above, only if PublicationActionParams.referrerProfileIds is not empty, the action can be successfully executed. This obviously does not conform to the design.

Tools Used

Manual Review

Recommended Mitigation Steps

File: contracts\modules\act\seadrop\SeaDropMintPublicationAction.sol
228: 
229:         uint256 referrersCut = (mintPaymentAmount * collectionData.referrersFeeBps) / MAX_BPS;
230:         
231:         uint256 referrersQuantity = processActionParams.referrerProfileIds.length;
232:---      uint256 feePerReferrer = referrersCut / referrersQuantity;
232:+++      uint256 feePerReferrer = referrersQuantity > 0 ? (referrersCut / referrersQuantity) : 0;
233: 
234:         if (feePerReferrer > 0) {

Assessed type

Math

donosonaumczuk commented 1 year ago

This haven't been tested yet, this modules contracts are out of scope, specified in the provided documentation. We kept them in the repo for context, so wardens can understand how the flows work with proper examples.

We appreciate the finding.

c4-sponsor commented 1 year ago

donosonaumczuk marked the issue as sponsor disputed

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Out of scope