Canto Identity Subprotocols contest - juancito's results

Subprotocols for Canto Identity Protocol.

General Information

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

Canto Identity Subprotocols

Findings Distribution

Researcher Performance

Rank: 27/98

Findings: 1

Award: $103.65

🌟 Selected for report: 1

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: juancito

Also found by: Chom, J4de, Ruhum, adriro, igingu, leopoldjoy, luxartvinsec, pipoca, popular00, reassor

Labels

bug
2 (Med Risk)
primary issue
selected for report
sponsor acknowledged
M-07

Awards

103.6509 USDC - $103.65

External Links

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L148-L168

Vulnerability details

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."

Impact

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.

Proof of Concept

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:     }

Link to code

Tools Used

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.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter