Cyfrin / 2023-08-sparkn

Other
11 stars 15 forks source link

Some tokens allow a winner to DoS at `distribute` #916

Open codehawks-bot opened 1 year ago

codehawks-bot commented 1 year ago

Some tokens allow a winner to DoS at distribute

Severity

Medium Risk

Relevant GitHub Links

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

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

Summary

Some tokens such as USDC, USDT, JPYC have a blocklist, if some of those addresses is included in a winner distribution, due to any reason such as a Supporter submission using one of these addresses as the value, it will create a DoS

Vulnerability Details

We can see how the distribution is basically made:

function distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
        external
    {
        if (msg.sender != FACTORY_ADDRESS) {
            revert Distributor__OnlyFactoryAddressIsAllowed();
        }
        _distribute(token, winners, percentages, data);
    }

And:

function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
    ...
    for (uint256 i; i < winnersLength; ) {
        uint256 amount = (totalAmount * percentages[i]) / BASIS_POINTS;
        erc20.safeTransfer(winners[i], amount); //@audit can revert
        unchecked {
            ++i;
        }
    }
    ...

In cases of tokens with blacklist, if one of the winners is a blacklisted address, it would block the whole distribution functioning, reverting all the previous loop iterations. Additionally there are some pauseable tokens that can block the system in similar way

### PoC: This test file includes a tests for erc20 tokens with block list, and with tokens that revert on transfers of 0 value. ```solidity // SPDX-License-Identifier: MIT pragma solidity 0.8.18; import {MockERC20} from "../test/mock/MockERC20.sol"; import {ECDSA} from "openzeppelin/utils/cryptography/ECDSA.sol"; import {Test, console} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; import {ProxyFactory} from "../src/ProxyFactory.sol"; import {Proxy} from "../src/Proxy.sol"; import {Distributor} from "../src/Distributor.sol"; import {HelperContract} from "../test/integration/HelperContract.t.sol"; import {Script} from "forge-std/Script.sol"; import {HelperConfig} from "script/HelperConfig.s.sol"; contract DeployContracts is Script { // tokens' array to whitelist address public stadiumAddress = makeAddr("stadium"); address public factoryAdmin = makeAddr("factoryAdmin"); function run() external returns (ProxyFactory, Distributor, HelperConfig) { // set up config HelperConfig config = new HelperConfig(); // get the addresses of the tokens to whitelist ( address jpycv1Address, address jpycv2Address, address usdcAddress, , uint256 deployerKey ) = config.activeNetworkConfig(); // whitelist 3 kinds of tokens address[] memory tokensToWhitelist = new address[](5); tokensToWhitelist[0] = jpycv1Address; tokensToWhitelist[1] = jpycv2Address; tokensToWhitelist[2] = usdcAddress; // address(uint160(uint256(keccak256(abi.encodePacked("No Zeros ERC20"))))) tokensToWhitelist[3] = 0xF9ee2486f2Aaf8245FFf41Ff18f40776dCc11a6d; // address(uint160(uint256(keccak256(abi.encodePacked("Banlist ERC20"))))) tokensToWhitelist[4] = 0xAcB40B1fBFFD23F4A99F32F12662B92B2E0c37C2; vm.startBroadcast(deployerKey); // prank ProxyFactory proxyFactory = new ProxyFactory(tokensToWhitelist); proxyFactory.transferOwnership(factoryAdmin); Distributor distributor = new Distributor( address(proxyFactory), stadiumAddress ); vm.stopBroadcast(); return (proxyFactory, distributor, config); } } contract MyHelperContract is HelperContract { constructor() { DeployContracts deployContracts = new DeployContracts(); (proxyFactory, distributor, config) = deployContracts.run(); ( jpycv1Address, jpycv2Address, usdcAddress, usdtAddress, deployerKey ) = config.activeNetworkConfig(); } } contract NOZEROSERC20 is MockERC20 { error NOZEROSERC20__TransferAmountMustBeMoreThanZero(); constructor() MockERC20("NOZEROSERC20", "NZS") {} // Modified from https://github.com/d-xo/weird-erc20/blob/main/src/RevertZero.sol function transferFrom( address from, address to, uint256 value ) public override returns (bool) { if (value == 0) { revert NOZEROSERC20__TransferAmountMustBeMoreThanZero(); } return super.transferFrom(from, to, value); } function transfer( address to, uint256 value ) public override returns (bool) { if (value == 0) { revert NOZEROSERC20__TransferAmountMustBeMoreThanZero(); } return super.transfer(to, value); } } contract BANERC20 is MockERC20 { error BANERC20__AccountBlacklisted(); mapping(address => bool) internal blacklisted; event Banned(address indexed _account); event UnBanned(address indexed _account); constructor() MockERC20("BANERC20", "BAN") {} modifier notBanned(address _account) { if (blacklisted[_account]) { revert BANERC20__AccountBlacklisted(); } _; } function isBanned(address _account) external view returns (bool) { return blacklisted[_account]; } function Ban(address _account) external { blacklisted[_account] = true; emit Banned(_account); } function unBan(address _account) external { blacklisted[_account] = false; emit UnBanned(_account); } function transferFrom( address from, address to, uint256 value ) public override notBanned(msg.sender) notBanned(from) notBanned(to) returns (bool) { return super.transferFrom(from, to, value); } function transfer( address to, uint256 value ) public override notBanned(msg.sender) notBanned(to) returns (bool) { return super.transfer(to, value); } } contract ProxyFactoryTest is StdCheats, MyHelperContract { // address(uint160(uint256(keccak256(abi.encodePacked("No Zeros ERC20"))))) address noZerosERC20 = 0xF9ee2486f2Aaf8245FFf41Ff18f40776dCc11a6d; address banlistERC20 = 0xAcB40B1fBFFD23F4A99F32F12662B92B2E0c37C2; function setUp() public { // set up balances of each token belongs to each user if (block.chainid == 31337) { // deal ether vm.deal(factoryAdmin, STARTING_USER_BALANCE); vm.deal(sponsor, SMALL_STARTING_USER_BALANCE); vm.deal(organizer, SMALL_STARTING_USER_BALANCE); vm.deal(user1, SMALL_STARTING_USER_BALANCE); vm.deal(user2, SMALL_STARTING_USER_BALANCE); vm.deal(user3, SMALL_STARTING_USER_BALANCE); vm.deal(TEST_SIGNER, SMALL_STARTING_USER_BALANCE); // mint erc20 token vm.etch(noZerosERC20, address(new NOZEROSERC20()).code); vm.etch(banlistERC20, address(new BANERC20()).code); deal(noZerosERC20, sponsor, 100_000 ether); deal(noZerosERC20, sponsor, 300_000 ether); deal(noZerosERC20, organizer, 100_000 ether); deal(noZerosERC20, organizer, 300_000 ether); deal(noZerosERC20, TEST_SIGNER, 100_000 ether); deal(noZerosERC20, TEST_SIGNER, 300_000 ether); deal(banlistERC20, sponsor, 100_000 ether); deal(banlistERC20, sponsor, 300_000 ether); deal(banlistERC20, organizer, 100_000 ether); deal(banlistERC20, organizer, 300_000 ether); deal(banlistERC20, TEST_SIGNER, 100_000 ether); deal(banlistERC20, TEST_SIGNER, 300_000 ether); } // labels vm.label(organizer, "organizer"); vm.label(sponsor, "sponsor"); vm.label(supporter, "supporter"); vm.label(user1, "user1"); vm.label(user2, "user2"); vm.label(user3, "user3"); } modifier setUpContestForJasonAndSentToken( address _organizer, address token ) { vm.startPrank(factoryAdmin); bytes32 randomId = keccak256(abi.encode("Jason", "001")); proxyFactory.setContest( _organizer, randomId, block.timestamp + 8 days, address(distributor) ); vm.stopPrank(); bytes32 salt = keccak256( abi.encode(_organizer, randomId, address(distributor)) ); address proxyAddress = proxyFactory.getProxyAddress( salt, address(distributor) ); vm.startPrank(sponsor); MockERC20(token).transfer(proxyAddress, 10000 ether); vm.stopPrank(); assertEq(MockERC20(token).balanceOf(proxyAddress), 10000 ether); _; } function createData(address token) public view returns (bytes memory data) { address[] memory tokens_ = new address[](1); tokens_[0] = token; address[] memory winners = new address[](2); winners[0] = user1; winners[1] = user2; uint256[] memory percentages_ = new uint256[](2); percentages_[0] = 9500; percentages_[1] = 0; data = abi.encodeWithSelector( Distributor.distribute.selector, token, winners, percentages_, "" ); } function testDOSWhenZeroPercentOnUser() public setUpContestForJasonAndSentToken(organizer, noZerosERC20) { // before assertEq(MockERC20(noZerosERC20).balanceOf(user1), 0 ether); assertEq(MockERC20(noZerosERC20).balanceOf(user2), 0 ether); assertEq(MockERC20(noZerosERC20).balanceOf(stadiumAddress), 0 ether); bytes32 randomId_ = keccak256(abi.encode("Jason", "001")); bytes memory data = createData(noZerosERC20); vm.warp(9 days); // 9 days later vm.startPrank(organizer); vm.expectRevert(ProxyFactory.ProxyFactory__DelegateCallFailed.selector); proxyFactory.deployProxyAndDistribute( randomId_, address(distributor), data ); vm.stopPrank(); } function testSucessWithBanlistToken() public setUpContestForJasonAndSentToken(organizer, banlistERC20) { // before assertEq(MockERC20(banlistERC20).balanceOf(user1), 0 ether); assertEq(MockERC20(banlistERC20).balanceOf(user2), 0 ether); assertEq(MockERC20(banlistERC20).balanceOf(stadiumAddress), 0 ether); bytes32 randomId_ = keccak256(abi.encode("Jason", "001")); bytes memory data = createData(banlistERC20); vm.warp(9 days); // 9 days later vm.startPrank(organizer); proxyFactory.deployProxyAndDistribute( randomId_, address(distributor), data ); vm.stopPrank(); // after assertEq(MockERC20(banlistERC20).balanceOf(user1), 9500 ether); assertEq(MockERC20(banlistERC20).balanceOf(user2), 0 ether); assertEq(MockERC20(banlistERC20).balanceOf(stadiumAddress), 500 ether); } function testDOSWhenBannedUser() public setUpContestForJasonAndSentToken(organizer, banlistERC20) { // before assertEq(MockERC20(banlistERC20).balanceOf(user1), 0 ether); assertEq(MockERC20(banlistERC20).balanceOf(user2), 0 ether); assertEq(MockERC20(banlistERC20).balanceOf(stadiumAddress), 0 ether); // ban user1 BANERC20(banlistERC20).Ban(user1); bytes32 randomId_ = keccak256(abi.encode("Jason", "001")); bytes memory data = createData(banlistERC20); vm.warp(9 days); // 9 days later vm.startPrank(organizer); vm.expectRevert(ProxyFactory.ProxyFactory__DelegateCallFailed.selector); proxyFactory.deployProxyAndDistribute( randomId_, address(distributor), data ); vm.stopPrank(); } } ``` The tests are using expectRevert and passing as expected: ```bash Running 3 tests for tcustom/PoCs.t.sol:ProxyFactoryTest PASS testDOSWhenBannedUser() (gas: 210440) PASS testDOSWhenZeroPercentOnUser() (gas: 205907) PASS testSucessWithBanlistToken() (gas: 238620) ```

Impact

DoS at distribute

Tools Used

Manual Review

Recommendations