Revolution Protocol - Ryonen's results

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/AuctionHouse.sol#L313 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/AuctionHouse.sol#L328 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/AuctionHouse.sol#L311

Vulnerability details

Impact

Under certain circumstances (either because the NFT to be minted has enough creators or because the NFT has content related to elevated metadata), there is a possibility for any malicious user to execute the AuctionHouse::settleCurrentAndCreateNewAuction function with sufficient gas for it to fail within try verbs.mint() due to gas insufficiency and continue with the remaining gas in catch {_pause();}.

Proof of Concept

This would be possible since, to complete the execution of the function AuctionHouse::_createAuction with a pause, the transaction should have approximately 23k gas to complete it. This would imply that 63/64 must be spent within try verbs.mint(), i.e., 1_500_000 gas at the moment of entering try verbs.mint(). This would be possible the more data-loaded the NFT is or also the more creators it has.

Tools Used

Foundry

Recommended Mitigation Steps

Perhaps a script to unpause the contract offchain could be useful, even if it's a dirty solution

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/AuctionHouse.sol#L309 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/AuctionHouse.sol#L328 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/AuctionHouse.sol#L265-L271

Vulnerability details

Impact

Whether with malicious intent or not, a user can create an NFT design good enough to win in voting but heavy enough in data for settleCurrentAndCreateNewAuction to use more than 30M gas to complete. This can also take advantage of the fact that _settleAuction is heavy in terms of gas consumed, even in cases where the contract is paused and unpause is used since _createAuction is called immediately. Due to consuming more than 30M gas, the contract would either revert or re-pause.

Proof of Concept

      ICultureIndex.ArtPieceMetadata memory metadata = ICultureIndex.ArtPieceMetadata("VERY HEAVY NFT")

        ICultureIndex.CreatorBps[] memory creators = new ICultureIndex.CreatorBps[](100);
        for(uint160 i = 1; i <= creators.length; i++){
            creators[i-1] = ICultureIndex.CreatorBps({ creator: address(i), bps: 100 });
        }
        uint256 pregasCreatePiece = gasleft();
        uint256 pieceId = cultureIndex.createPiece(metadata, creators);
        console.log(pregasCreatePiece - gasleft());
        console.log("Gas consumed create piece");

        vm.startPrank(address(erc20TokenEmitter));
        erc20Token.mint(address(this), 10);

        vm.roll(block.number + 1);

        vm.startPrank(address(this));
        cultureIndex.vote(pieceId); 

        vm.startPrank(address(auction));
        uint256 preGas = gasleft();
        uint256 tokenId = erc721Token.mint();
        uint256 postGas = gasleft();
        console.log(preGas - postGas);
        console.log("Gas consumed: ");

Additionally, it should be demonstrated that mint is heavier than createPiece.

Gas consumed create piece: 29_087_428

Gas consumed mint: 30_154_233

Tools Used

Foundry

Recommended Mitigation Steps

Implementing a gas limit in the createPiece function would be advisable.

Assessed type

DoS