[Audit Report] Blur

0xDave·2022년 12월 13일
TILblockchaincode4renaethereum
1

Ethereum

목록 보기
74/112
post-thumbnail

Blur는 최근 에어드랍으로 핫한 패러다임의 NFT 마켓플레이스 프로젝트다. 오딧 리포트를 잠깐 봤더니 High 1개, Medium 1개 있어서 간단하게 정리해보려 한다.

Architecture

전체적인 구조를 살펴보면, 3가지 메인 컴포넌트로 구성된 프록시 형태다. MatchingPolicy에서 주문을 매칭해주면, 해당 주문을 Exchange 컨트랙트가 받아와서 ExecutionDelegate에 넘겨주고 이후 토큰과 NFT를 교환해주는 구조다.

1) Exchange
2) MatchingPolicy
3) ExecutionDelegate


주요 결함

[H-01] STANDARDPOLICYERC1155.SOL RETURNS AMOUNT == 1 INSTEAD OF AMOUNT == ORDER.AMOUNT

//BlurExchange.sol

    function execute(Input calldata sell, Input calldata buy)
        external
        payable
        reentrancyGuard
        whenOpen
    {
        require(sell.order.side == Side.Sell);

        bytes32 sellHash = _hashOrder(sell.order, nonces[sell.order.trader]);
        bytes32 buyHash = _hashOrder(buy.order, nonces[buy.order.trader]);

        require(_validateOrderParameters(sell.order, sellHash), "Sell has invalid parameters");
        require(_validateOrderParameters(buy.order, buyHash), "Buy has invalid parameters");

        require(_validateSignatures(sell, sellHash), "Sell failed authorization");
        require(_validateSignatures(buy, buyHash), "Buy failed authorization");

        (uint256 price, uint256 tokenId, uint256 amount, AssetType assetType) = _canMatchOrders(sell.order, buy.order);

        _executeFundsTransfer(
            sell.order.trader,
            buy.order.trader,
            sell.order.paymentToken,
            sell.order.fees,
            price
        );
        _executeTokenTransfer(
            sell.order.collection,
            sell.order.trader,
            buy.order.trader,
            tokenId,
            amount,
            assetType
        );

문제가 발생하는 부분은 _canMatchOrders다. sell과 buy 주문을 매칭해서 거래에 필요한 항목들을 리턴해준다. 아래 _canMatchOrders 함수를 자세히 보면 amount를 무조건 1로 리턴하게 되어있다. 이렇게 되면 주문이 어떻게 들어오든지 buyer는 ERC1155 토큰 1개만 받게 된다.

//StandardPolicyERC1155.sol

    function canMatchMakerAsk(Order calldata makerAsk, Order calldata takerBid)
        external
        pure
        override
        returns (
            bool,
            uint256,
            uint256,
            uint256,
            AssetType
        )
    {
        return (
            (makerAsk.side != takerBid.side) &&
            (makerAsk.paymentToken == takerBid.paymentToken) &&
            (makerAsk.collection == takerBid.collection) &&
            (makerAsk.tokenId == takerBid.tokenId) &&
            (makerAsk.matchingPolicy == takerBid.matchingPolicy) &&
            (makerAsk.price == takerBid.price),
            makerAsk.price,
            makerAsk.tokenId,
            1,
            AssetType.ERC1155
        );
    }

테스트 코드

    it('Only 1 ERC1155 received for order with amount > 1', async () => {
      await mockERC1155.mint(alice.address, tokenId, 10); //10개 민팅
      //판매 주문 생성
      sell = generateOrder(alice, {
        side: Side.Sell,
        tokenId,
        amount: 10,
        collection: mockERC1155.address,
        matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
      });
      //구매 주문 생성
      buy = generateOrder(bob, {
        side: Side.Buy,
        tokenId,
        amount: 10,
        collection: mockERC1155.address,
        matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
      });
      
      //execute 함수에 들어갈 calldata 생성
      sellInput = await sell.pack();
      buyInput = await buy.pack();
      
      //거래 트랜잭션 생성
      await waitForTx(exchange.execute(sellInput, buyInput));
      
      // Buyer는 1개의 토큰만 받는다.
      expect(await mockERC1155.balanceOf(bob.address, tokenId)).to.be.equal(1);
      await checkBalances(
        aliceBalance,
        aliceBalanceWeth.add(priceMinusFee),
        bobBalance,
        bobBalanceWeth.sub(price),
        feeRecipientBalance,
        feeRecipientBalanceWeth.add(fee),
      );
    });

중간에 테스트 코드가 이해가지 않아서 다른 부분을 좀 살펴봤다. 먼저 이해가 안 갔던 부분은 pack()이다. hardhat이나 ehters 문서를 찾아봐도 나오지 않아서 vscode로 repo를 가져와 검색해봤다. pack()은 테스트 코드에 따로 있었다. generateOrder도 컨트랙트가 아니라 테스트 코드에서 만든 함수였다. generateOrder에 거래 데이터를 입력해주면 Order라는 클래스를 리턴해준다. Order 클래스 안에 있는 메소드가 pack()이었다. waitForTx도 이런 식이었다.

//utils.ts

  async pack(
    options: { signer?: Signer; oracle?: Signer; blockNumber?: number } = {},
  ) {
    const signature = await sign(
      this.parameters,
      options.signer || this.user,
      this.exchange,
    );
    return {
      order: this.parameters,
      v: signature.v,
      r: signature.r,
      s: signature.s,
      extraSignature: packSignature(
        await oracleSign(
          this.parameters,
          options.oracle || this.admin,
          this.exchange,
          options.blockNumber ||
            (
              await ethers.provider.getBlock('latest')
            ).number,
        ),
      ),
      signatureVersion: 0,
      blockNumber: (await ethers.provider.getBlock('latest')).number,
    };
  }
//setup.ts

  const generateOrder = (account: Wallet, overrides: any = {}): Order => {
    //Order는 class
    return new Order(
      account,
      {
        trader: account.address,
        side: Side.Buy,
        matchingPolicy: matchingPolicies.standardPolicyERC721.address,
        collection: mockERC721.address,
        tokenId,
        amount: 0,
        paymentToken: weth.address,
        price,
        listingTime: '0',
        expirationTime: '0',
        fees: [
          {
            rate: feeRate,
            recipient: thirdParty.address,
          },
        ],
        salt: 0,
        extraParams: '0x',
        ...overrides,
      },
      admin,
      exchange,
    );
  };

[M-01] CONTRACT OWNER POSSESSES TOO MANY PRIVILEGES

현재 Architecture를 보면 ExecutionDelegate에 유저들의 자산을 approve하게 되어있다. 여기서 주장하는 결함은 ExecutionDelegate에 너무 많은 권한이 부여돼있고, 프로젝트 측에서 러그풀 하지 않을거라는 신뢰만으로 approve하기엔 나이브한 면이 있다는 점. 프로젝트 운영진이 마음만 먹으면 러그풀이 손 쉽게 가능하다는 점이다.

//ExecutionDelegate.sol

function transferERC20(address token, address from, address to, uint256 amount)
        approvedContract
        external
        returns (bool)
    {
        require(revokedApproval[from] == false, "User has revoked approval");
        return IERC20(token).transferFrom(from, to, amount);
    }

function approveContract(address _contract) onlyOwner external {
        contracts[_contract] = true;
        emit ApproveContract(_contract);
    }

function revokeApproval() external {
        revokedApproval[msg.sender] = true;
        emit RevokeApproval(msg.sender);
    }

자산을 이동시키기 위해선 approvedContract에 등록된 컨트랙트여야 한다. 컨트랙트 오너는 approvedContract에 다른 컨트랙트를 등록시킬 수 있고, 유저들은 나중에 revokeApproval()를 통해 컨트랙트에게 위임한 권한을 다시 가져올 수 있다. 어떤 특이 케이스더라도 일단 ExecutionDelegate 컨트랙트의 owner 권한이 탈취되면 막대한 자금이 손실될 것 이다. 따라서 프로토콜의 구조를 다시 디자인 하는 것이 바람직하다.


배운 것

  1. 테스트 코드를 작성할 때는 프로젝트 측에서 앞서 작성해 놓은 테스트 코드를 미리 살펴보자. ethers나 hardhat에 없는 함수를 만들어 놓은 경우도 있으니 적극 활용하자.

  2. 하드코딩된 부분을 발견하면 의심부터 할 것. 해당 코드로부터 의도와 다른 결과물을 리턴하는 건 없는지 알아볼 것.

  3. 하나의 컨트랙트에 너무 많은 권한이 위임되진 않았는지 확인하기.


출처 및 참고자료

  1. Blur Exchange contest Findings & Analysis Report
profile
0xDave
Just BUIDL :)
이전 포스트

[Ethereum] All about Assembly

다음 포스트

[Code4rena] Forgeries

0개의 댓글