[Code4rena] Escher

0xDave·2022년 12월 10일

Ethereum

목록 보기

72/112

Escher is a decentralized curated marketplace for editioned artwork

Scope

File	SLOC	Description and Coverage	Libraries
Contracts (12)
src/uris/Unique.sol	9	Metadata Contract, 0.00%	`@openzeppelin/*`
src/uris/Base.sol	15	Metadata Contract, 0.00%	`@openzeppelin-upgradeable/*`
src/uris/Generative.sol 🧮	20	Metadata Contract, 0.00%
src/Escher721Factory.sol 🌀	28	Token Factory, 0.00%	`@openzeppelin/*`
src/minters/FixedPriceFactory.sol 🌀	31	Sale Factory, 100.00%	`@openzeppelin/*`
src/minters/OpenEditionFactory.sol 🌀	31	Sale Factory, 100.00%	`@openzeppelin/*`
src/minters/LPDAFactory.sol 🌀	35	Sale Factory, 100.00%	`@openzeppelin/*`
src/Escher.sol 🧮	48	Manages roles, 80.00%	`@openzeppelin/*`
src/Escher721.sol 🧮	64	ERC721 Token, 15.38%	`@openzeppelin-upgradeable/*`
src/minters/FixedPrice.sol 💰 💣 📤	66	Sale Contract, 82.61%	`@openzeppelin-upgradeable/*`
src/minters/OpenEdition.sol 💰 💣 📤	81	Sale Contract, 81.48%	`@openzeppelin-upgradeable/*`
src/minters/LPDA.sol 💰 📤	106	Sale Contract, 91.11%	`@openzeppelin-upgradeable/*`
Total (over 12 files):	534	72.67%

전체적인 흐름

먼저 결함을 살펴보기 전에 전체적인 구조를 살펴보자. 사람들이 Escher라는 nft 마켓플레이스에 와서 curator 또는 creator로 활동할 수 있다. curator는 사람들에게 creator 롤을 부여할 수 있고 creator는 자신의 nft를 판매할 수 있다. 판매하는 방식은 크게 3종류로 나뉜다.

Fixed price (지정가)
Open edition (endTime이 있음)
LPDA (Last Price Dutch Auction)

이후 세일이 끝나면 feeReceiver에게 5% 수수료를 전송하고 끝.

내가 생각한 결함

1. FixedPrice나 OpenEdition으로 했을 경우, 세일 끝나고 fund를 영원히 잃을 수 있음.

//FixedPrice.sol

contract FixedPrice is Initializable, OwnableUpgradeable, ISale {
    struct Sale {
        // slot 1
        uint48 currentId;
        uint48 finalId;
        address edition;
        // slot 2
        uint96 price;
        address payable saleReceiver;
        // slot 3
        uint96 startTime;
    }

     constructor() {
        factory = msg.sender;
        _disableInitializers();
        sale = Sale(0, 0, address(0), type(uint96).max, payable(0), type(uint96).max); //처음에 0으로 되어있는거 인지하자.
    }
  
    //..
  
    function _end(Sale memory _sale) internal {
        emit End(_sale);
        ISaleFactory(factory).feeReceiver().transfer(address(this).balance / 20);
        selfdestruct(_sale.saleReceiver);
    }

}

//FixedPriceFactory.sol

    function createFixedSale(FixedPrice.Sale calldata _sale) external returns (address clone) {
        require(IEscher721(_sale.edition).hasRole(bytes32(0x00), msg.sender), "NOT AUTHORIZED");
        require(_sale.startTime >= block.timestamp, "START TIME IN PAST");
        require(_sale.finalId > _sale.currentId, "FINAL ID BEFORE CURRENT");

        clone = implementation.clone();
        FixedPrice(clone).initialize(_sale);

        emit NewFixedPriceContract(msg.sender, _sale.edition, clone, _sale);
    }

FixedPriceFactory를 통해서 Sale을 만들 때, require문에 saleReceiver가 제대로 들어가 있는지 확인하는 코드가 없다. 나중에 세일이 끝나면 saleReceiver에게 세일금이 가게 되어있는데, 이 때 주소가 처음 constructor에서 세팅된 payable(0) 값으로 되어 있다면 세일금을 찾지 못 할 것이다. 이는 OpenEdition에서도 똑같이 발생할 수 있다. 따라서 Factory 컨트랙트에 아래와 같은 require문을 추가해야 한다.

require(sale.saleReceiver != address(0), "INVALID SALE RECEIVER");

2. LPDA 세일에서 finalPrice가 startPrice보다 높을 경우, 세일이 안 끝나서 자금이 묶이는 문제 발생

//LPDAFactory.sol

    function createLPDASale(LPDA.Sale calldata sale) external returns (address clone) {
        require(IEscher721(sale.edition).hasRole(bytes32(0x00), msg.sender), "NOT AUTHORIZED");
        require(sale.saleReceiver != address(0), "INVALID SALE RECEIVER");
        require(sale.startTime >= block.timestamp, "INVALID START TIME");
        require(sale.endTime > sale.startTime, "INVALID END TIME");
        require(sale.finalId > sale.currentId, "INVALID FINAL ID");
        require(sale.startPrice > 0, "INVALID START PRICE");
        require(sale.dropPerSecond > 0, "INVALID DROP PER SECOND");

        clone = implementation.clone();
        LPDA(clone).initialize(sale);

        emit NewLPDAContract(msg.sender, sale.edition, clone, sale);
    }

LPDA는 더치옥션으로 세일이 진행되는 방식이다. 그런데 Factory 컨트랙트에 finalPrice가 startPrice보다 낮다는 require문이 없다. 이렇게 되면 다음과 같은 문제가 발생한다.

//LPDA.sol

    function getPrice() public view returns (uint256) {
        Sale memory temp = sale;
        (uint256 start, uint256 end) = (temp.startTime, temp.endTime);
        if (block.timestamp < start) return type(uint256).max;
        if (temp.currentId == temp.finalId) return temp.finalPrice;

        uint256 timeElapsed = end > block.timestamp ? block.timestamp - start : end - start;
        return temp.startPrice - (temp.dropPerSecond * timeElapsed);
    }

    function cancel() external onlyOwner {
        require(block.timestamp < sale.startTime, "TOO LATE");
        sale.finalId = sale.currentId;
        _end();
    }

LPDA 세일이 시작되면 해당 nft의 가격은 getPrice()를 통해 정해진다. 만약 현재 판매하고 있는 nft의 id가 마지막 id라면 가격은 finalPrice를 반영한다. 다음과 같은 시나리오를 생각할 수 있다.

finalPrice가 startPrice보다 높게 세일이 형성된다.
세일이 진행될수록 세일가는 계속 낮아진다.(dropPerSecond 때문에)
마지막 id 직전의 nft는 가장 낮은가격에 판매가 된다.
마지막 id가 팔릴 차례가 되면 가격이 최고가가 된다. (finalPrice가 가장 높게 설정되어 있기 때문)
사람들은 가장 비싼 가격에 형성되어있는 것을 보고 더 이상 nft를 구매하지 않는다.
마지막 id가 팔리지 않는다면 세일은 끝나지 않는다.
세일이 끝나지 않으면 판매금이 컨트랙트에 계속 묶이고 판매자는 영영 판매금을 받지 못한다.
이를 인지한 판매자가 cancel하려 해보지만 이미 세일이 시작돼서 cancel 하지도 못 한다.

해결 방법으로 두 가지가 필요하다. 첫 번째는 finalPrice가 startPrice보다 낮다는 require문이 필요하다.

require(sale.startPrice > sale.finalPrice, "INVALID START PRICE");

두 번째는 endTime이 지나면 세일이 자동으로 끝나게 해야 한다. 현재 endTime은 계속해서 떨어지는 가격을 계산할 때만 사용된다. 따라서 endTime이 지나도 세일이 끝나지 않는 것을 방지할 필요가 있다.

3. dropPerSecond가 높을 경우, 세일이 진행되지 않음

//LPDAFactory.sol

    function createLPDASale(LPDA.Sale calldata sale) external returns (address clone) {
        require(IEscher721(sale.edition).hasRole(bytes32(0x00), msg.sender), "NOT AUTHORIZED");
        require(sale.saleReceiver != address(0), "INVALID SALE RECEIVER");
        require(sale.startTime >= block.timestamp, "INVALID START TIME");
        require(sale.endTime > sale.startTime, "INVALID END TIME");
        require(sale.finalId > sale.currentId, "INVALID FINAL ID");
        require(sale.startPrice > 0, "INVALID START PRICE");
        require(sale.dropPerSecond > 0, "INVALID DROP PER SECOND");

        clone = implementation.clone();
        LPDA(clone).initialize(sale);

        emit NewLPDAContract(msg.sender, sale.edition, clone, sale);
    }

현재 dropPerSecond가 0보다 높은지만 확인한다. 만약 dropPerSecond가 충분히 높을 경우, 가격을 가져올 수 없는 문제가 생긴다.

//LPDA.sol

    function getPrice() public view returns (uint256) {
        Sale memory temp = sale;
        (uint256 start, uint256 end) = (temp.startTime, temp.endTime);
        if (block.timestamp < start) return type(uint256).max;
        if (temp.currentId == temp.finalId) return temp.finalPrice;

        uint256 timeElapsed = end > block.timestamp ? block.timestamp - start : end - start;
        return temp.startPrice - (temp.dropPerSecond * timeElapsed);
    }

마지막 계산식에서 temp.dropPerSecond * timeElapsed 값이 temp.startPrice 보다 클 경우 음수가 되는데 이 경우 getPrice()를 호출했을 때 revert가 난다. getPrice()가 제대로 동작하지 않는다면 buy() 함수도 호출할 수 없다. 구매자체가 불가능하니 세일은 더 이상 진행되지 않는다. 따라서 아래와 같은 if문이 필요하다.

if (temp.startPrice < (temp.dropPerSecond * timeElapsed)) return temp.finalPrice;

4. currentId가 0인 nft는 민팅되지 않는다.

    function buy(uint256 _amount) external payable {
        Sale memory sale_ = sale;
        IEscher721 nft = IEscher721(sale_.edition);
        require(block.timestamp >= sale_.startTime, "TOO SOON");
        require(_amount * sale_.price == msg.value, "WRONG PRICE");
        uint48 newId = uint48(_amount) + sale_.currentId;
        require(newId <= sale_.finalId, "TOO MANY");

        for (uint48 x = sale_.currentId + 1; x <= newId; x++) {
            nft.mint(msg.sender, x);
        }

        sale.currentId = newId;

        emit Buy(msg.sender, _amount, msg.value, sale);

        if (newId == sale_.finalId) _end(sale);
    }

FixedPrice에서 currentId는 초기에 무조건 0이다. 다른 판매방식은 Max값이지만 크게 상관은 없다. 만약 creator가 판매하는 NFT의 tokenId가 0인 작품이 있다면 민팅되지 않고 세일이 끝날 것이다. 따라서 판매자에게 currentId가 0인 nft를 만들지 말라고 사전에 알릴 필요가 있다.

새로 알게 된 것

Clones

contract FixedPriceFactory is Ownable {
    using Clones for address;
  	//..
}

각 Factory 컨트랙트에 Clones 라이브러리를 적용했다. Clones는 openZeppelin에서 제공하는 라이브러리로, proxy 컨트랙트를 배포할 때 가스를 절약하기 위해 사용한다. 아래 예시를 든 것 처럼 유니스왑의 페어처럼 매우 빈번하게 proxy를 사용할 때 유용하다.

EIP-1155

erc1155는 erc20과 erc721을 섞은 것으로 게임 아이템 종류별로 묶어서 하나의 컨트랙트를 구성한다고 보면 된다. 각각의 아이템마다 컨트랙트가 하나씩 필요하다면 나중에 거래할 때 매우 번거로우며 가스비 낭비도 심하다. 이 때 erc1155를 사용하면 적은 트랜잭션으로 여러개의 아이템을 이동시키거나 민팅할 수 있다.

    function safeTransferFrom(
        address from,
        address to,
        uint256 id,
        uint256 amount,
        bytes memory data
    ) public virtual override {
        require(
            from == _msgSender() || isApprovedForAll(from, _msgSender()),
            "ERC1155: caller is not token owner or approved"
        );
        _safeTransferFrom(from, to, id, amount, data);
    }

    function _safeTransferFrom(
        address from,
        address to,
        uint256 id,
        uint256 amount,
        bytes memory data
    ) internal virtual {
        require(to != address(0), "ERC1155: transfer to the zero address");

        address operator = _msgSender();
        uint256[] memory ids = _asSingletonArray(id);
        uint256[] memory amounts = _asSingletonArray(amount);

        _beforeTokenTransfer(operator, from, to, ids, amounts, data);

        uint256 fromBalance = _balances[id][from];
        require(fromBalance >= amount, "ERC1155: insufficient balance for transfer");
        unchecked {
            _balances[id][from] = fromBalance - amount;
        }
        _balances[id][to] += amount;

        emit TransferSingle(operator, from, to, id, amount);

        _afterTokenTransfer(operator, from, to, ids, amounts, data);

        _doSafeTransferAcceptanceCheck(operator, from, to, id, amount, data);
    }

erc1155에서 토큰을 전송할 수 있는 함수는 2가지가 있다. safeTransferFrom과 safeBatchTransferFrom. 그리고 setApprovalForAll을 통해 외부인이 해당 함수들을 호출할 수 있도록 권한을 넘길 수 있다.

후기

확실히 ParaSpace 때 많은 LOC를 겪다보니 이번 Scope의 양은 적게 느껴졌다. 메인 로직을 좀 더 집중해서 고민했었고 그 결과 결함들을 몇 개 찾을 수 있었다. (물론 레포트에 추가될 지는 모른다..) 좋은 결과가 있으면 좋겠다. 콘테스트가 계속 쏟아져나와서 이번주는 정신 없이 코드만 봐야될 것 같다.