Revolution Protocol - plasmablocks's results

Lines of code

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

Vulnerability details

Impact

The ERC20TokenEmitter:buyToken() method allows users to purchase protocol governance tokens with native eth. The eth payment is split between protocol rewards, the protocol treasury and the creatorAddress in that order. After protocol rewards are paid the remaining eth msgValueRemaining is split between the treasury and the creatorAddress.

The amount of eth sent to the creatorAddress is determined by the creatorRateBps and entropyRateBps. If entropyRateBps is not set to 100% then there will be eth remaining in the ERC20TokenEmitter contract at the end of the transaction. The amount of eth that will be locked can be determined by:

uint256 excessEth = msgValueRemaining - toPayTreasury - creatorDirectPayment

• This will result in some amount of eth remaining in the contract on every buyToken() call.

• The ERC20TokenEmitter.sol contract does not have a withdraw() method so funds will be locked.

In the fuzz test below, the amount of excessEth locked varies from < 1% of passed in eth to > 95% of passed in eth. The excessEth grows proportionately with msg.value and creatorRateBps and grows inversely with entropyRateBps.

Proof of Concept

Below I have written a PoC in foundry showing ETH being locked. This test can be added to the ERC20TokenEmitter.t.sol test suite:

function testFuzzLeftOverEthInTokenEmitterContract(uint256 creatorRateBps, uint256 entropyRateBps, uint256 amountOfEther) public {

        vm.assume(creatorRateBps < 10000 && entropyRateBps < 10000);
        vm.assume(creatorRateBps > 0 && entropyRateBps > 500);

        vm.assume(amountOfEther > 1 ether);
        vm.assume(amountOfEther < 50 ether);


        vm.startPrank(address(dao));
        erc20TokenEmitter.setCreatorRateBps(creatorRateBps);
        erc20TokenEmitter.setCreatorRateBps(entropyRateBps);
        vm.stopPrank();

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

        // Correct total of 10,000 basis points (100%)
        uint256[] memory correctBps = new uint256[](2);
        correctBps[0] = 5000; // 50%
        correctBps[1] = 5000; // 50%

        // Eth is locked even if these addresses are the 0 address
        address builderAddress = address(0x1424);
        address purchaseReferralAddress = address(0x1948);
        address deployerAddress = address(0x1521);

        address caller = address(0x48324);

        vm.deal(caller, 51 ether);
        vm.startPrank(caller);

        int expectedAmount = erc20TokenEmitter.getTokenQuoteForEther(amountOfEther - erc20TokenEmitter.computeTotalReward(amountOfEther));
        assertGt(expectedAmount, 0, "Token purchase should have a positive amount");

        // Attempting a valid token purchase
        uint emittedWad = erc20TokenEmitter.buyToken{ value: amountOfEther }(
            recipients,
            correctBps,
            IERC20TokenEmitter.ProtocolRewardAddresses({
                builder: builderAddress,
                purchaseReferral: purchaseReferralAddress,
                deployer: deployerAddress
            })
        );

        assertLt(address(erc20TokenEmitter).balance, amountOfEther / 2, "Large portion of eth sent to token emitter is locked in contract");
        assertLt(address(erc20TokenEmitter).balance, 0.1 ether, "Ether locked in contract");
    }

Tools Used

Foundry

Recommended Mitigation Steps

There are a few different changes that can be made depending on the protocol desired outcome:

• After transferring funds to protocol rewards, the protocol treasury and creatorAddress ensure any excess eth is refunded to the msg.sender or in the case of the AuctionHouse settlement, refund the auction.bidder.

• Potentially add a withdraw() method to the ERC20TokenEmitter contract so that eth can't be locked.

• Potentially add off-chain monitoring to the balance of the ERC20TokenEmitter contract so the protocol team can be alerted if eth is locked in the contract and potentially pause the auction house to prevent further loses.

Assessed type

ETH-Transfer