Summary
The mintHorse() implementation in HorseStore.huff prevents the minting of more than one horse. The issue arises from the failure to update the TOTAL_SUPPLY variable after each minting operation. Consequently, the same tokenId value is used in subsequent minting attempts, triggering a revert with the error message ALREADY_MINTED.
Vulnerability Details
Test Case
To reproduce the test, paste the function inside HorseStoreHuff.t.sol::HorseStoreHuff, then run forge test --mt testFail_HuffCantMintMultipleHorses -vvvv in the terminal
function testFail_HuffCantMintMultipleHorses() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.prank(user);
horseStore.mintHorse();
vm.prank(user2);
horseStore.mintHorse();
vm.prank(user3);
horseStore.mintHorse();
}
Logs
Running 1 test for test/HorseStoreHuff.t.sol:HorseStoreHuff
[PASS] testFail_HuffCantMintMultipleHorses() (gas: 65832)
Traces:
[65832] HorseStoreHuff::testFail_HuffCantMintMultipleHorses()
├─ [
0]
VM::
addr(<pk>) [staticcall]
│ └─ ← user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802]
├─ [0] VM::label(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], "user2")
│ └─ ← ()
├─ [
0]
VM::
addr(<pk>) [staticcall]
│ └─ ← user3: [0xc0A55e2205B289a967823662B841Bd67Aa362Aec]
├─ [0] VM::label(user3: [0xc0A55e2205B289a967823662B841Bd67Aa362Aec], "user3")
│ └─ ← ()
├─ [0] VM::prank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [50931] 0x6d2eed85750d316088343D6d5e91ca59eb052768::mintHorse()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 0)
│ └─ ← ()
├─ [0] VM::prank(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← ()
├─ [501] 0x6d2eed85750d316088343D6d5e91ca59eb052768::mintHorse()
│ └─ ← revert: ALREADY_MINTED
└─ ← revert: ALREADY_MINTED
Impact
The protocol becomes unusable after a user mints a horse, as subsequent minting attempts fail, leading to the inability to mint new horses.
Tools Used
Manual review
Recommendations
Load TOTAL_SUPPLY value from storage in MINT_HORSE()
#define macro MINT_HORSE() = takes (0) returns (0) {
[TOTAL_SUPPLY] // [TOTAL_SUPPLY]
+ sload // [totalSupply]
caller // [msg.sender, TOTAL_SUPPLY]
_MINT() // []
stop // []
}
Implement a macro to update TOTAL_SUPPLY
+#define macro _UPDATE_TOTAL_SUPPLY() = takes (0) returns (0) {
+ [TOTAL_SUPPLY] // [TOTAL_SUPPLY]
+ sload // [totalSupply]
+ 0x01 add // [totalSupply+1]
+ [TOTAL_SUPPLY] // [TOTAL_SUPPLY]
+ sstore // []
+}
Update total supply in _MINT()
#define macro _MINT() = takes (2) returns (0) {
// Input stack: // [to, tokenId]
// Output stack: // []
// Check that the recipient is valid
dup1 iszero invalid_recipient jumpi // [to, tokenId]
// Create the minting params
0x00 dup3 // [tokenId, from (0x00), to, tokenId]
// Check token ownership
[OWNER_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [owner, from (0x00), to, tokenId]
unauthorized jumpi
// Give tokens to the recipient.
TRANSFER_GIVE_TO() // [from (0x00), to, tokenId]
// Emit the transfer event.
__EVENT_HASH(Transfer) // [sig, from (0x00), to, tokenId]
0x00 0x00 log4 // []
+ // Update the total supply
+ _UPDATE_TOTAL_SUPPLY() // []
// Continue Executing
cont jump
invalid_recipient:
INVALID_RECIPIENT(0x00)
unauthorized:
ALREADY_MINTED(0x00)
cont:
}
