Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Zero address check in the transfer loop.

inallhonesty

Summary

Funds can be lost by mistakenly sending them to the zero address. Moreover some tokens revert when attempting to transfer to address(0).

Vulnerability Details

In the winners array the organizer can set one of the winner's addresses to address(0). This can lead to accidentally sending some tokens to address(0) or, on some tokens, can cause a revert. In case of a revert the organizer can waste a lot of gas because all the transfers happen within a for loop.

Impact

Accidental burning of prizes or wasted gas.

Tools Used

Manual review

Recommendations

There are 2 ways of solving this:

  1. Add a check that skips the transfer transaction in case the winner address == 0.

uint256 winnersLength = winners.length; // cache length
for (uint256 i; i < winnersLength;) {
uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;
++ if (winners[i] == address(0)) continue;
erc20.safeTransfer(winners[i], amount);
unchecked {
++i;
}
}

Pro: The other winners receive their share of the prize, we minimize the wasted gas for the organizer.

Con: The remaining balance of the contract is sent to the owner through the _commissionTransfer(erc20). The owner would have to make the winner whole by spending some gas for the transfer.

  1. Add a check that reverts the whole transaction in case the winner address == 0

uint256 winnersLength = winners.length; // cache length
for (uint256 i; i < winnersLength;) {
uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;
++ if (winners[i] == address(0)) revert Distributor__NoZeroAddress();
erc20.safeTransfer(winners[i], amount);
unchecked {
++i;
}
}

Pro: There are no partial solutions. The organizer has to come up with the correct data for the transfers to go through.

Con: Organizer will waste gas which can amount to serious amounts due to transfers being done in a loop. Might refuse to call it again correctly leaving the problem in the hands of the owner.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.