기존 Treasure NFT Marketplace 취약점 분석
Treasure NFT marketplace
이전 TreasureDAO NFT 마켓플레이스를 예전에 분석했던 경험과, 이때 찾았던 취약점을 말해보고자 한다. 또한, 2022년 3월초에 실제로 해당 마켓플레이스 NFT 자산들이 탈취된 공격에 대해 설명해보고자 한다.
기존 마켓플이스의 전체 코드는 Arbiscan에서의 Contract에서 확인해볼 수 있다. 우선 내가 처음 파악했던 문제점은 NFT를 리스팅 할 때 ERC721과 ERC1155에 대한 quantity 체크를 명확히 하지 않는 부분에 있다. 아래 코드를 보자.
function createListing(
address _nftAddress,
uint256 _tokenId,
uint256 _quantity,
uint256 _pricePerItem,
uint256 _expirationTime
) external notListed(_nftAddress, _tokenId, _msgSender()) onlyWhitelisted(_nftAddress) {
if (_expirationTime == 0) _expirationTime = type(uint256).max;
require(_expirationTime > block.timestamp, "invalid expiration time");
require(_quantity > 0, "nothing to list");
if (IERC165(_nftAddress).supportsInterface(INTERFACE_ID_ERC721)) {
IERC721 nft = IERC721(_nftAddress);
require(nft.ownerOf(_tokenId) == _msgSender(), "not owning item");
require(nft.isApprovedForAll(_msgSender(), address(this)), "item not approved");
} else if (IERC165(_nftAddress).supportsInterface(INTERFACE_ID_ERC1155)) {
IERC1155 nft = IERC1155(_nftAddress);
require(nft.balanceOf(_msgSender(), _tokenId) >= _quantity, "must hold enough nfts");
require(nft.isApprovedForAll(_msgSender(), address(this)), "item not approved");
} else {
revert("invalid nft address");
}
listings[_nftAddress][_tokenId][_msgSender()] = Listing(
_quantity,
_pricePerItem,
_expirationTime
);
emit ItemListed(
_msgSender(),
_nftAddress,
_tokenId,
_quantity,
_pricePerItem,
_expirationTime
);
}
위 코드에서의 문제점은 ERC721에 대한 수량체크가 이뤄지고 있지 않다는 점이다. ERC1155는 아래와 같이 소유권을 확인함과 동시에, quantity만큼 보유하고 있는지 체크하며 자동으로 quantity에 대한 검사를 수행한다.
require(nft.balanceOf(_msgSender(), _tokenId) >= _quantity, "must hold enough nfts");
하지만 ERC721에 대한 아래 코드를 보면 알 수 있듯이, ERC721는 무조건 수량이 1이어야됨에도 불구하고 require(_quantity==1, quantity error)와 같은 에러 처리문이 없다. ERC721은 같은 NFT에 대해 수량이 존재하는 ERC1155과는 달리 하나하나가 고유한 NFT로서 quantity가 무조건 1이 되어야하는데, 단지 소유권만 확인하고 비정상적인 quantity에 대한 체크를 빼먹은 케이스이다.
if (IERC165(_nftAddress).supportsInterface(INTERFACE_ID_ERC721)) {
IERC721 nft = IERC721(_nftAddress);
require(nft.ownerOf(_tokenId) == _msgSender(), "not owning item");
require(nft.isApprovedForAll(_msgSender(), address(this)), "item not approved");
}
이 경우엔 Attacker가 1보다 높은 quantity를 사용하여 createListing 함수를 호출할 수 있어 예상치 못한 공격 포인트를 만들 수 있다.
그럼 실제 NFT를 구매하는 buyItems 함수를 살펴보자.
function buyItem(
address _nftAddress,
uint256 _tokenId,
address _owner,
uint256 _quantity
)
external
nonReentrant
isListed(_nftAddress, _tokenId, _owner)
validListing(_nftAddress, _tokenId, _owner)
{
require(_msgSender() != _owner, "Cannot buy your own item");
Listing memory listedItem = listings[_nftAddress][_tokenId][_owner];
require(listedItem.quantity >= _quantity, "not enough quantity");
// Transfer NFT to buyer
if (IERC165(_nftAddress).supportsInterface(INTERFACE_ID_ERC721)) {
IERC721(_nftAddress).safeTransferFrom(_owner, _msgSender(), _tokenId);
} else {
IERC1155(_nftAddress).safeTransferFrom(_owner, _msgSender(), _tokenId, _quantity, bytes(""));
}
if (listedItem.quantity == _quantity) {
delete (listings[_nftAddress][_tokenId][_owner]);
} else {
listings[_nftAddress][_tokenId][_owner].quantity -= _quantity;
}
emit ItemSold(
_owner,
_msgSender(),
_nftAddress,
_tokenId,
_quantity,
listedItem.pricePerItem
);
TreasureNFTOracle(oracle).reportSale(_nftAddress, _tokenId, paymentToken, listedItem.pricePerItem);
_buyItem(listedItem.pricePerItem, _quantity, _owner);
}
해당 함수의 경우 역시 ERC721에 대한 quantity 체크가 되어있지 않다. ERC721를 구매하고자 함수가 호출된 경우 수량이 1개인지 확인하는 require문이 필요했다. 해당 문제점은 2022년 2월 9일에 확인한 문제점인데 이 때에는 이런 수량문제가 크리티컬한 이슈로 번질지는 몰랐고, 수량을 체크하지 않음으로 인해 잠재적인 문제가 발생할 것이라는 점만 인지하였다.
예를 들어, 판매자가 CreateListing을 통해 ERC721을 quantity를 10으로 하여 등록한 경우를 가정해보자. 그럼 구매자가 buyItem으로 quantity를 1로해서 구매할 때, 전송과 결제엔 아무런 문제도 없지만 buyItem 함수의 아래 코드로 인하여 기존 구조체가 남겨져있는 문제가 발생한다.
if (listedItem.quantity == _quantity) {
delete (listings[_nftAddress][_tokenId][_owner]);
}
위 코드에선 구매자가 구매한 수량이 기존에 올라온 매물의 NFT 수량과 일치할 경우에만 기존 구조체 정보를 삭제하는데, 위의 시나리오에선 10 == 1 이 일하지 않아 delete문이 실행되지 않음으로써 쓰레기 구조체 정보가 남게된다.
실제 공격 취약점
위에서 발견한 ERC721에 대한 수량 체크 이슈는 2022년 2월 9일에 발견하여 해당 프로젝트의 Github Discussion에 제출하였었다. 하지만 크리티컬한 이슈로 이어질 것을 예상하지 못했기 때문에 해당 팀에서는 큰 이슈로 판단을 하지 않았었고, 이는 3월 3일 대규모 공격으로 이어졌다.
실제로 2022년 3월 3일 공격이 시행된 부분은 _buyItem에서 _quantity를 0으로 하여 돈을 지불하지 않고 NFT를 구매한 공격이 실행되었다. 이를 통해 약 100개 이상의 NFT가 탈취되었었다 (약 $1.4M 규모).
function _buyItem(
uint256 _pricePerItem,
uint256 _quantity,
address _owner
) internal {
uint256 totalPrice = _pricePerItem * _quantity;
uint256 feeAmount = totalPrice * fee / BASIS_POINTS;
IERC20(paymentToken).safeTransferFrom(_msgSender(), feeReceipient, feeAmount);
IERC20(paymentToken).safeTransferFrom(_msgSender(), _owner, totalPrice - feeAmount);
}
buyItem에서 내부적으로 호출되는 _buyItem 함수에선 구매자가 판매자에게 돈을 지불하는 로직이 수행된다. 여기서도 수량에 대한 별도의 체크를 하지 않는다. 여기서 quantity를 0으로 했을 때 totalPrice는 0이 되어 feeAmount가 0이되어 구매자는 어떠한 자금도 지불하지 않게된다. 이를 통해 ERC721 토큰에 대해서 돈을 지불하지 않고 탈취가 가능하게 됐다. ERC1155는 이 공격에서 안전했는데, 아래 코드에서 보듯이 buyItem 함수에서 NFT를 transfer하는 과정에서 quantity가 0일 경우 어떠한 NFT도 전송하지 않기 때문이다. 즉 해당 공격은 ERC721에 대한 비정상적인 (이 케이스에선 수량을 0으로 했을 때) quantity 수치를 예외처리 하지 못해 발생하였다.
// Transfer NFT to buyer
if (IERC165(_nftAddress).supportsInterface(INTERFACE_ID_ERC721)) {
IERC721(_nftAddress).safeTransferFrom(_owner, _msgSender(), _tokenId);
} else {
IERC1155(_nftAddress).safeTransferFrom(_owner, _msgSender(), _tokenId, _quantity, bytes(""));
}
해당 공격이 발생하고 나서 커뮤니티에선 왜 내가 이전에 제시한 버그 리포트를 무시했냐라는 비판이 나왔지만, 사실은 ERC721에 대한 수량체크 결여라는 부분이 같을 뿐 내가 작성한 [Dicussion]에서는 위와 같은 크리티컬한 이슈까지는 발견하지 못했다. 이 부분을 왜 발견하지 못했는지 정말 아쉬운 부분이다.
정리하자면, 단순히 ERC721에 대한 수량 체크 require문을 하나 빼먹을 뿐인데, 돈을 지불하지 않고 NFT를 구매할 수 있는 치명적인 버그로 이어졌다. 스마트 컨트렉트에서 가장 중요한 부분은 보안이라는 것을 다시 한번 느끼게 된 사건이다.
다행히도 탈취된 NFT들은 모두 원소유자들에게 반환되었고, 위에 설명한 취약점들은 현재 다 개선된 상태이다. TreasureDAO는 기존의 마켓플레이스를 Opensea와 같은 제너럴한 형태로 변형한 아비트럼의 대표 NFT 마켓플레이스인 Trove를 운영중이다. 이 취약점 발견 이후로 TreasureDAO는 Peer Review/Audit 절차를 모든 프로덕트에 필수적으로 적용하도록 제품 출시 프로세스를 변경하였다. 커뮤니티원들과 투자자들은 프로덕트의 빠른 출시를 원하지만, 위 사례와 같은 치명적인 공격을 예방하기 위해선 Audit을 필수적으로 선행해야한다고 본다.
번외, ERC721 Lead Author와의 만남
해당 버그 사태 이후에 ERC721의 Lead Author인 William Entriken가 수정된 마켓플레이스에 대해 코드 리뷰어로 참여했다. ERC721의 주저자라길래 어떤식으로 코드 리뷰를 하나 살펴봤는데, 기본적인 리뷰부터 심층적인 리뷰에 이르기까지 리뷰의 정석을 보는 느낌이였다. 개발의 룰모델로 삼고싶을 정도이다. 해당 리뷰에 관심있는 분들은 William Entriken의 full review를 참고해보자. 정말 많은 공부가 된다.
물론 나도 마켓플레이스 지불 토큰과 관련된 시큐리티 이슈를 하나 올려 기여를 하였다. 많이 부족하지만 앞으로 꾸준히 스마트 컨트렉트 분석/리뷰에 참여하며 실력을 키워나가고 싶다.
추후엔 Trove, Opensea, Looksrare, X2Y2와 같은 마켓플레이스에 대한 코드를 검토하고 각 마켓플레이스의 설계가 어떻게 다른지 살펴보도록 하겠다.