Platform: Code4rena
Start Date: 17/03/2023
Pot Size: $36,500 USDC
Total HM: 10
Participants: 98
Period: 3 days
Judge: leastwood
Total Solo HM: 5
Id: 223
League: ETH
Rank: 27/98
Findings: 1
Award: $103.65
🌟 Selected for report: 1
🚀 Solo Findings: 0
103.6509 USDC - $103.65
Trays and their tiles are the building blocks for fusing a Namespace NFT. The tiles in a tray are generated based on a hash of the previous mint.
Users will need specific tiles to be able to create the name they want, so it is of utter importance for them to get the trays they want with their corresponding tiles. The users should be able to precompute this, as the documentation states:
"The 7 tiles per tray are then generated according to a deterministic algorithm. A user can therefore precompute which trays he will get."
Users can pay the corresponding $NOTE tokens to buy a tray with some specific tiles, buy end up obtaining a completely different one with tiles they don't expect.
This can happen due to normal usage of the platform, with some user sending a transaction with more gas. Or by a malicious user frontrunning the transaction.
On any case the user ends up with an NFT he didn't want instead of reverting the transaction, and saving their $NOTE tokens.
There is no check on the buy()
function to make sure that the user will actually buy the tray he wanted, thus possibly obtaining another one.
The tiles are calculated based on a hash. That hash is generated each time a new mint is produced, and it is based on the last one.
If the transaction is frontrunned, the user will get a new hash, and thus new tiles data for his tray.
File: canto-namespace-protocol/src/Tray.sol 148: /// @notice Buy a specifiable amount of trays 149: /// @param _amount Amount of trays to buy 150: function buy(uint256 _amount) external { 151: uint256 startingTrayId = _nextTokenId(); 152: if (prelaunchMinted == type(uint256).max) { 153: // Still in prelaunch phase 154: if (msg.sender != owner) revert OnlyOwnerCanMintPreLaunch(); 155: if (startingTrayId + _amount - 1 > PRE_LAUNCH_MINT_CAP) revert MintExceedsPreLaunchAmount(); 156: } else { 157: SafeTransferLib.safeTransferFrom(note, msg.sender, revenueAddress, _amount * trayPrice); 158: } 159: for (uint256 i; i < _amount; ++i) { 160: TileData[TILES_PER_TRAY] memory trayTiledata; 161: for (uint256 j; j < TILES_PER_TRAY; ++j) { 162: lastHash = keccak256(abi.encode(lastHash)); // @audit-info hash is calculated based on the last one 163: trayTiledata[j] = _drawing(uint256(lastHash)); // @audit-info tiles data is calculated based on the hash 164: } 165: tiles[startingTrayId + i] = trayTiledata; 166: } 167: _mint(msg.sender, _amount); // We do not use _safeMint on purpose here to disallow callbacks and save gas 168: }
Manual review
Check the hash for the last minted Tray NFT, to make sure it is the same the user used to precalculate the next tiles.
On the case the transaction is frontrunned, the expected hash will be different and the transaction will revert, saving the user from spending the $NOTE tokens.
// File: canto-namespace-protocol/src/Tray.sol /// @notice Buy a specifiable amount of trays /// @param _amount Amount of trays to buy - function buy(uint256 _amount) external { + function buy(uint256 _amount, bytes32 _lastHash) external { + require(_lastHash == lastHash, "Hashes must be the same"); uint256 startingTrayId = _nextTokenId(); if (prelaunchMinted == type(uint256).max) { // Still in prelaunch phase if (msg.sender != owner) revert OnlyOwnerCanMintPreLaunch(); if (startingTrayId + _amount - 1 > PRE_LAUNCH_MINT_CAP) revert MintExceedsPreLaunchAmount(); } else { SafeTransferLib.safeTransferFrom(note, msg.sender, revenueAddress, _amount * trayPrice); } for (uint256 i; i < _amount; ++i) { TileData[TILES_PER_TRAY] memory trayTiledata; for (uint256 j; j < TILES_PER_TRAY; ++j) { lastHash = keccak256(abi.encode(lastHash)); trayTiledata[j] = _drawing(uint256(lastHash)); } tiles[startingTrayId + i] = trayTiledata; } _mint(msg.sender, _amount); // We do not use _safeMint on purpose here to disallow callbacks and save gas }
#0 - c4-sponsor
2023-03-29T20:53:22Z
OpenCoreCH marked the issue as sponsor acknowledged
#1 - 0xleastwood
2023-04-10T14:44:02Z
While this could be treated as a duplicate of #121 , I like how the warden has provided a remediation which would protect users from being front-run and receiving an unexpected ID.
#2 - c4-judge
2023-04-10T14:44:42Z
0xleastwood marked the issue as primary issue
#3 - 0xleastwood
2023-04-10T20:31:13Z
@OpenCoreCH Would be interested to hear your perspective on this. There are a tonne of issues detailing the predictability of tray buys. However, only a select few detail a remediation which improves user experience and is still in-line with what was stated in the contest README. Does it make sense to keep these issues as medium severity and downgrade the rest to QA or should we keep all the issues as one but only give partial credit to the poorer quality issues?
#4 - c4-judge
2023-04-11T19:43:59Z
0xleastwood marked the issue as selected for report
#5 - OpenCoreCH
2023-04-11T22:15:52Z
@OpenCoreCH Would be interested to hear your perspective on this. There are a tonne of issues detailing the predictability of tray buys. However, only a select few detail a remediation which improves user experience and is still in-line with what was stated in the contest README. Does it make sense to keep these issues as medium severity and downgrade the rest to QA or should we keep all the issues as one but only give partial credit to the poorer quality issues?
Hmm good question. I'd say giving partial credit to the poorer quality issues makes sense, as they address in principle the same issue.