search

code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

[M1] Incorrect amount of gas sent in `_distributeFunds` #268

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/Constants.sol#L45-L48 https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L130

Vulnerability details

Impact

In case recipients consume more gas than expected the transaction could revert or cost can be too high.

Proof of Concept

According to the definition of the variable SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS you intend to use all that gas for all recipients.

/** 
@dev The gas limit to send ETH to multiple recipients, enough for a 5-way split.
*/
 uint256 constant SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS = 210000;

However, you are sending that amount for every recipient.

 for (uint256 i = 0; i < creatorRecipients.length; ++i) {
    _sendValueWithFallbackWithdraw(
      creatorRecipients[i],
      creatorShares[i],
      SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS
    );

For the case of 5 recipients if the fallback function consumes lot of gas you will need a million of gas for this loop!.

Recommended

 [-] for (uint256 i = 0; i < creatorRecipients.length; ++i) {
    _sendValueWithFallbackWithdraw(
      creatorRecipients[i],
      creatorShares[i],
      SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS
    );

[+] for (uint256 i = 0; i < creatorRecipients.length; ++i) {
   _sendValueWithFallbackWithdraw(
   creatorRecipients[i],
   creatorShares[i],
   SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS / MAX_ROYALTY_RECIPIENTS
 );
peritoflores commented 1 year ago

I wanted to send this as "medium".

HardlyDifficult commented 1 year ago

Dupe of https://github.com/code-423n4/2022-08-foundation-findings/issues/165