NextGen - alexxander's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L196-L253 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L189-L200 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L227-L232

Vulnerability details

Impact

Both allow list & public users can circumvent the max allowance of tokens allowed to be minted per address. Merkle tree allowance is bypassed.

Proof of Concept

The allowlist case is considered since a merkle tree bounds the address space (in public minting anyone can just mint from different addresses, although, circumvanting turns out to be possible on per address basis as well).

require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, msg.sender) + _numberOfTokens, "AL limit");

The problem is that the minter per address counter in NextGenCore contract is updated after the NFT is minted in _mintProcessing(...)

_mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);

            if (phase == 1) {

                tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;

            } else {

                tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
            }

This is dangerous since a malicious user can use the onERC721Received hook to reenter mint(...) in Minter contract before the tokensMintedAllowlistAddress is updated. This ultimately allows the allow list user to mint more tokens than what is stated in the Merkle Tree. The same goes for public minting as well. Also, an interesting caveat is that even if the allow list consists only of EOA addresses, a malicious user can still use the delegate feature to delegate towards a malicious contract that will mint on his behalf.

Coded POC

• Run forge init --force
• Place the code in the gist in a test file under /test
• POC Gist - https://gist.github.com/alexxander77/ce41e149979bc07aa4220de6970212bc
• Run with forge test --match-test testBypassMaxAllowance
• Output - 10 minted tokens for user1, although the allowance is 5 (see assertions)

Tools Used

Manual Inspection

Recommended Mitigation Steps

Mint after the allowance counter update

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L57-L129

Vulnerability details

Impact

A malicious user can use a Flash Loan to drain all the ongoing auctions except the one he uses to execute the attack from.

Proof of Concept

This attack is build upon several issues contained within AuctionDemo.sol. The listed issues have smaller impact on their own, however, this report explains how they can be combined to cause maximum damage.

An auction starts from mintAndAuction(…) where the NFT is minted to some “Custody” contract that approves to AuctionDemo contract and auctionEndTime is passed as the time that the auction will end. From that point anyone can call particpateToAuction(…) in AuctionDemo to submit a bid. Users can also cancel their bids or wait till claimAuction(…) is called when the Auction has ended and everyone, except the winner, will get a refund of their bids, while the winner receives the NFT. It’s also important to note that there is only 1 AuctionDemo contract that can host arbitrary amount of Auctions for any collection for any duration i.e we have auctions running in parallel.

Issue #1

claimAuction(…) is available at auctionEndTime, however, participateToAuction(…) is also available at auctionEndTime. This causes a front run concern where whoever is faster to beat the best bid at the auctionEndTime block.timestamp can immediately call claimAuction and win. The only precondition is that auctionEndTime indeed falls on a block.timestamp, which is likely since we can have multiple auctions.

function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
	// more code ...
}

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
}

Issue #2

When refunding bidders, claimAuction(…) doesn’t update auctionInfoData[_tokenid][index].status, which means that a bidder can implement a fallback() function that, upon receiving the bid refund, calls cancelBid(…) and gets a second refund for the same bid i.e double spending issue. This is under the assumption that claimAuction is called exactly at auctionEndTime and auctionEndTime falls on a block.timestamp, otherwise, cancelBid(…) is not available.

else if (auctionInfoData[_tokenid][i].status == true) {
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);

Exploit

Assuming that auctionEndTime falls on a block.timestamp for an auction that's ongoing, a malicious user can formulate the following attack

note

• exploitableBalance is the balance of the AuctionDemo contract (i.e all auctions) minus the amount that has to be refunded to the bidders in the auction that the attack is launched from
• Malicious user deploys a contract that has a onERC721Received hook & a programmed fallback() function (this can be done prior to the attack block)
• Everything happens in 1 transaction that is front run to ensure success

1. At auctionEndTime user acquires into his malicious contract a flash loan worth exploitableBalance + a highest bid (to be able bid & to call claimAuction(...))
2. Malicious contract calls participateToAuction(...) with a highest bid
3. Malicious contract calls claimAuction(...), winning the auction
4. claimAuction(...) will now refund any refundees and send the NFT to the malicious contract
5. onERC721Received is triggered inside the Malicious contract - inside the hook Malicious contract calls participateToAuction(...) with a new bid that is worth exactly exploitableBalance
6. The execution returns to the claimAuction(...) invocation where we now have an updated state [auctionInfoData that is iterated over has one more entry] with one more bid from the malicious contract.
7. Now claimAuction(...) proceeds to refund the Malicious contract for exploitableBalance and enters maliciou's contract fallback()
8. In the fallback() Malicious contract calls cancelBid(…) and receives a second refund worth exploitableBalance i.e stealing all the funds inside AuctionDemo from all other ongoing auctions
9. Malicious contract returns the flashloan of 1*exploitableBalance + highestBid
10. Malicious contract profits 1*exploitableBalance - highestBid + NFT

Coded POC

• Run forge init --force
• Place the code in the gist in a test file under /test
• POC Gist - https://gist.github.com/alexxander77/6caa5f969a936137b5ced3b2548831c0
• run with forge test --match-test testRobAuctions -vv
• Auction Contract has 100 ether + 6 ether from bidders inside the auction that the attack is launched from. Attacker starts with 0 ether
• Attacker ends up with 96 ether - 1 wei

Tools Used

Manual Inspection

Recommended Mitigation Steps

Ensure claimAuction(...), participateToAuction(...) & cancelBid(...) cannot be called at the same block.timestamp

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L104-L120

Vulnerability details

Impact

A contract that wins an auction can postpone owner & refundee’s payments indefinitely.

Proof of Concept

At the time of winning, an Auction holds the owner’s sell fee (equal to what the winning bid is) and also holds extra funds from the non-winning bids. Upon concluding an auction it is expected that the owner & refundees receive their payments. To distribute these fees either an Admin or the winner of the auction has to call claimAuction(...). The issue is that a malicious actor can bid & win through a contract that has a reverting onERC721Received() hook function. In that case claimAuction(...) will revert when it attempts to safeTransferFrom(...) the NFT to the malicious bidder. In such a manner a malicious winner can have final authority when and if the owner & refundees get their payments. Note that this does require financial commitment from the malicious winner (i.e he has to win the auction), however the funds that will be withheld will be more than what he commits.

Additional Note

The same idea of bricking an auction’s funds and NFT but at significantly cheaper cost (eg 1 wei + gas fees) can be executed by a malicious bidder that front runs the first bid through a contract that implements a “return bomb” (memory expansion & copy attack) in the fallback() function - this will cause claimAuction(...) to revert when it attempts to refund the 1 wei bidder. This finding however will most likely be deemed OOS because of [L-18] in the bot findigs, nevertheless, I am noting it here for sponsor’s & judge reference since it implies severe consequneces.

Coded POC

• Run forge init --force
• Place the code from the gist in a test file under /test
• POC Gist - https://gist.github.com/alexxander77/0cca44dea3d670e08bd72aaf4d905d35
• run with forge test --match-test testForcedRevert
• Output - claimAuction(...) reverts

Tools Used

Manual Inspection

Recommended Mitigation Steps

Separate the logic of claiming the NFT, paying the owner & refunding refundees.

Assessed type

DoS