Skip to content

Commit

Permalink
Hotfix/convert auction currency vuln (#29)
Browse files Browse the repository at this point in the history
* chore: working test of vuln

* fix currency pass through, ensure no bid exists, check owner on settle

* chore: cleaned up test structure

* feat: wip prevent locked bids on nefarious nfts

* feat: added check to see if auction had started

---------

Co-authored-by: koloz <[email protected]>
  • Loading branch information
charlescrain and koloz193 authored Aug 31, 2023
1 parent a979633 commit 2e80cb5
Show file tree
Hide file tree
Showing 6 changed files with 484 additions and 31 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ build: ## Build the smart contracts with foundry.

.PHONY: test
test: ## Run foundry unit tests.
@bash -l -c 'forge test'
@bash -l -c 'forge test --no-match-path src/test/forks/**/*.sol'
68 changes: 38 additions & 30 deletions src/auctionhouse/SuperRareAuctionHouse.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,33 @@ contract SuperRareAuctionHouse is
_checkSplits(_splitAddresses, _splitRatios);
_checkValidAuctionType(_auctionType);

require(_lengthOfAuction <= maxAuctionLength, "configureAuction::Auction too long.");
{
require(_lengthOfAuction <= maxAuctionLength, "configureAuction::Auction too long.");

Auction memory auction = tokenAuctions[_originContract][_tokenId];
Auction memory auction = tokenAuctions[_originContract][_tokenId];

require(
auction.auctionType == NO_AUCTION || auction.auctionCreator != msg.sender,
"configureAuction::Cannot have a current auction."
);
Bid memory staleBid = auctionBids[_originContract][_tokenId];

require(_lengthOfAuction > 0, "configureAuction::Length must be > 0");
require(staleBid.bidder == address(0), "configureAuction::bid shouldnt exist");

if (_auctionType == COLDIE_AUCTION) {
require(_startingAmount > 0, "configureAuction::Coldie starting price must be > 0");
} else if (_auctionType == SCHEDULED_AUCTION) {
require(_startTime > block.timestamp, "configureAuction::Scheduled auction cannot start in past.");
}
require(
auction.auctionType == NO_AUCTION || (auction.auctionCreator != msg.sender),
"configureAuction::Cannot have a current auction"
);

require(
_startingAmount <= marketplaceSettings.getMarketplaceMaxValue(),
"configureAuction::Cannot set starting price higher than max value."
);
require(_lengthOfAuction > 0, "configureAuction::Length must be > 0");

if (_auctionType == COLDIE_AUCTION) {
require(_startingAmount > 0, "configureAuction::Coldie starting price must be > 0");
} else if (_auctionType == SCHEDULED_AUCTION) {
require(_startTime > block.timestamp, "configureAuction::Scheduled auction cannot start in past.");
}

require(
_startingAmount <= marketplaceSettings.getMarketplaceMaxValue(),
"configureAuction::Cannot set starting price higher than max value."
);
}

tokenAuctions[_originContract][_tokenId] = Auction(
payable(msg.sender),
Expand Down Expand Up @@ -130,6 +136,11 @@ contract SuperRareAuctionHouse is
"convertOfferToAuction::Cannot have a current auction."
);

require(
auction.startingTime == 0 || block.timestamp < auction.startingTime,
"convertOfferToAuction::Auction must not have started."
);

require(_lengthOfAuction <= maxAuctionLength, "convertOfferToAuction::Auction too long.");

Offer memory currOffer = tokenCurrentOffers[_originContract][_tokenId][_currencyAddress];
Expand Down Expand Up @@ -204,6 +215,8 @@ contract SuperRareAuctionHouse is
erc721.transferFrom(address(this), msg.sender, _tokenId);
}

require(erc721.ownerOf(_tokenId) == msg.sender, "sending failed");

emit CancelAuction(_originContract, _tokenId, auction.auctionCreator);
}

Expand Down Expand Up @@ -330,7 +343,7 @@ contract SuperRareAuctionHouse is
_payout(
_originContract,
_tokenId,
auction.currencyAddress,
currBid.currencyAddress,
currBid.amount,
auction.auctionCreator,
auction.splitRecipients,
Expand All @@ -340,6 +353,8 @@ contract SuperRareAuctionHouse is
marketplaceSettings.markERC721Token(_originContract, _tokenId, true);
}

require(erc721.ownerOf(_tokenId) == currBid.bidder, "sending failed");

emit AuctionSettled(
_originContract,
currBid.bidder,
Expand All @@ -356,21 +371,14 @@ contract SuperRareAuctionHouse is
/** @return Auction Struct: creatorAddress, creationTime, startingTime, lengthOfAuction,
currencyAddress, minimumBid, auctionType, splitRecipients array, and splitRatios array.
*/
function getAuctionDetails(address _originContract, uint256 _tokenId)
function getAuctionDetails(
address _originContract,
uint256 _tokenId
)
external
view
override
returns (
address,
uint256,
uint256,
uint256,
address,
uint256,
bytes32,
address payable[] memory,
uint8[] memory
)
returns (address, uint256, uint256, uint256, address, uint256, bytes32, address payable[] memory, uint8[] memory)
{
Auction memory auction = tokenAuctions[_originContract][_tokenId];

Expand All @@ -392,4 +400,4 @@ contract SuperRareAuctionHouse is
revert("Invalid Auction Type");
}
}
}
}
242 changes: 242 additions & 0 deletions src/test/bazaar/SuperRareBazaar.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.0;

import {Test, console2} from "forge-std/Test.sol";

import {ISuperRareBazaar, SuperRareBazaar} from "../../bazaar/SuperRareBazaar.sol";
import {ISuperRareMarketplace, SuperRareMarketplace} from "../../marketplace/SuperRareMarketplace.sol";
import {IMarketplaceSettings} from "rareprotocol/aux/marketplace/IMarketplaceSettings.sol";
import {IStakingSettings} from "rareprotocol/aux/marketplace/IStakingSettings.sol";
import {IRoyaltyRegistry} from "rareprotocol/aux/registry/interfaces/IRoyaltyRegistry.sol";
import {IPayments} from "rareprotocol/aux/payments/IPayments.sol";
import {Payments} from "rareprotocol/aux/payments/Payments.sol";
import {ISpaceOperatorRegistry} from "rareprotocol/aux/registry/interfaces/ISpaceOperatorRegistry.sol";
import {IApprovedTokenRegistry} from "rareprotocol/aux/registry/interfaces/IApprovedTokenRegistry.sol";
import {IRoyaltyEngineV1} from "royalty-registry/IRoyaltyEngineV1.sol";
import {Payments} from "rareprotocol/aux/payments/Payments.sol";
import {ISuperRareAuctionHouse, SuperRareAuctionHouse} from "../../auctionhouse/SuperRareAuctionHouse.sol";
import {IRareStakingRegistry} from "../../staking/registry/IRareStakingRegistry.sol";
import {IERC721} from "openzeppelin-contracts/token/ERC721/IERC721.sol";
import {IERC20} from "openzeppelin-contracts/token/ERC20/IERC20.sol";
import {SuperFakeNFT} from "../../test/utils/SuperFakeNFT.sol";
import {TestRare} from "../../test/utils/TestRare.sol";



contract SuperRareBazaarTest is Test {
TestRare private superRareToken;
SuperRareMarketplace private superRareMarketplace;
SuperRareAuctionHouse private superRareAuctionHouse;
SuperRareBazaar private superRareBazaar;


address marketplaceSettings = address(0xabadaba1);
address royaltyRegistry = address(0xabadaba2);
address royaltyEngine = address(0xabadaba3);
address spaceOperatorRegistry = address(0xabadaba6);
address approvedTokenRegistry = address(0xabadaba7);
address stakingRegistry = address(0xabadaba9);
address networkBeneficiary = address(0xabadabaa);
address rewardPool = address(0xcccc);

address private immutable exploiter = vm.addr(0x123);
address private immutable exploiter1 = vm.addr(0x231);
address private immutable bidder = vm.addr(0x321);

uint256 private constant TARGET_AMOUNT = 249.6 ether;

uint256 private constant _lengthOfAuction = 1;

bytes32 private constant SCHEDULED_AUCTION = "SCHEDULED_AUCTION";

SuperFakeNFT private sfn;

function setUp() public {
// Create market, auction, bazaar, and token contracts
superRareToken = new TestRare();
superRareMarketplace = new SuperRareMarketplace();
superRareAuctionHouse = new SuperRareAuctionHouse();
superRareBazaar = new SuperRareBazaar();

// Deploy Payments
Payments payments = new Payments();

// Initialize the bazaar
superRareBazaar.initialize(marketplaceSettings, royaltyRegistry, royaltyEngine, address(superRareMarketplace), address(superRareAuctionHouse), spaceOperatorRegistry, approvedTokenRegistry, address(payments), stakingRegistry, networkBeneficiary);

SuperFakeNFT _sfn = new SuperFakeNFT(address(superRareBazaar));
sfn = _sfn;

sfn.mint(exploiter, 1);
superRareToken.transfer(bidder, 300 ether);
vm.deal(address(superRareBazaar), 300 ether);

vm.prank(bidder);
superRareToken.approve(address(superRareBazaar), type(uint256).max);

vm.prank(exploiter);
sfn.setApprovalForAll(address(superRareBazaar), true);

vm.prank(exploiter1);
sfn.setApprovalForAll(address(superRareBazaar), true);

// etch code into these so we can stub out methods. Need some
vm.etch(marketplaceSettings, address(superRareToken).code);
vm.etch(stakingRegistry, address(superRareToken).code);
vm.etch(royaltyRegistry, address(superRareToken).code);
vm.etch(royaltyEngine, address(superRareToken).code);
vm.etch(spaceOperatorRegistry, address(superRareToken).code);
vm.etch(approvedTokenRegistry, address(superRareToken).code);
}

function test_auctions_with_eth_sucess() public {

}

function test_auctions_with_erc20_success() public {

}

function test_convert_offer_currency_exploit() external {

/*///////////////////////////////////////////////////
Mock Calls
///////////////////////////////////////////////////*/
vm.mockCall(
stakingRegistry,
abi.encodeWithSelector(IRareStakingRegistry.getRewardAccumulatorAddressForUser.selector, exploiter1),
abi.encode(address(0))
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IStakingSettings.calculateMarketplacePayoutFee.selector, TARGET_AMOUNT),
abi.encode((TARGET_AMOUNT * 3) / 100)
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IStakingSettings.calculateStakingFee.selector, TARGET_AMOUNT),
abi.encode(0)
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IMarketplaceSettings.getMarketplaceFeePercentage.selector),
abi.encode(3)
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IMarketplaceSettings.getMarketplaceMaxValue.selector),
abi.encode(type(uint256).max)
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IMarketplaceSettings.calculateMarketplaceFee.selector, TARGET_AMOUNT),
abi.encode((TARGET_AMOUNT * 3) / 100)
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IMarketplaceSettings.hasERC721TokenSold.selector, address(sfn), 1),
abi.encode(false)
);
vm.mockCall(
spaceOperatorRegistry,
abi.encodeWithSelector(ISpaceOperatorRegistry.isApprovedSpaceOperator.selector, exploiter1),
abi.encode(false)
);
vm.mockCall(
approvedTokenRegistry,
abi.encodeWithSelector(IApprovedTokenRegistry.isApprovedToken.selector, address(superRareToken)),
abi.encode(true)
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IMarketplaceSettings.getERC721ContractPrimarySaleFeePercentage.selector, address(sfn)),
abi.encode(15)
);
vm.mockCall(
marketplaceSettings,
abi.encodeWithSelector(IMarketplaceSettings.markERC721Token.selector, address(sfn)),
abi.encode()
);

/*///////////////////////////////////////////////////
Test
///////////////////////////////////////////////////*/
configureAuction();

skip(12); //~about 1 block
vm.expectRevert();
superRareBazaar.settleAuction(address(sfn), 1);
}


/*//////////////////////////////////////////////////////////////////////////
Helper Functions
//////////////////////////////////////////////////////////////////////////*/

// Receive function for test contract to be sent value
receive() external payable {
console2.log("Amount Recieved by Attacker:", msg.value);
}

// Configure the auction
function configureAuction() internal {
// Setup the Offer and convert it to an auction
createOfferAndConvertToAuction();

address payable[] memory _splitAddresses = new address payable[](1);
_splitAddresses[0] = payable(address(this));

uint8[] memory _splitRatios = new uint8[](1);
_splitRatios[0] = 100;

//@exploit: Assumes all NFTs follows the ERC-721 spec
vm.prank(exploiter);
IERC721(sfn).transferFrom(exploiter, exploiter1, 1);

//@exploit: Overwrites previously set auction with a new Currency (ETH). Keeps the same bid
vm.prank(exploiter1);
vm.expectRevert();
superRareBazaar.configureAuction(
SCHEDULED_AUCTION,
address(sfn),
1,
TARGET_AMOUNT,
address(0),
_lengthOfAuction,
block.timestamp + 1,
_splitAddresses,
_splitRatios
);
}

function createOfferAndConvertToAuction() internal {
createOffer();

address payable[] memory _splitAddresses = new address payable[](1);
_splitAddresses[0] = payable(address(this));

uint8[] memory _splitRatios = new uint8[](1);
_splitRatios[0] = 100;

vm.prank(exploiter);
superRareBazaar.convertOfferToAuction(
address(sfn),
1,
address(superRareToken),
TARGET_AMOUNT,
_lengthOfAuction,
_splitAddresses,
_splitRatios
);
}

function createOffer() internal {
console2.log("Before Attack: SuperRareBazaar ETH Balance:", address(superRareBazaar).balance);

//@exploit: Create an Offer using a custom NFT and the superRareToken as Currency
vm.prank(bidder);
superRareBazaar.offer(address(sfn), 1, address(superRareToken), TARGET_AMOUNT, true);
}


}
Loading

0 comments on commit 2e80cb5

Please sign in to comment.