First Flight #7: Horse Store
First Flight #7
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Fallback in `HorseStore.huff` calls `GET_TOTAL_SUPPLY`

n0kto

Description

In the MAIN macro, no revert was placed before labels. Consequently, for any call to a non-existing function, it will execute GET_TOTAL_SUPPLY instead of reverting.

#define macro MAIN() = takes (0) returns (0) {
.
.
.
dup1 __FUNC_SIG(ownerOf) eq ownerOf jumpi
​
@>
​
totalSupply:
GET_TOTAL_SUPPLY()
.
.
.

Impact

Any user or contract can misinterpret the return data and mistakenly believe that the function they called exists. This behavior is not compliant with Solidity contracts.

Proof of Concept

Foundry PoC
function testFallbackReturnTotalSupply() public {
(bool sent, bytes memory data) = address(horseStore).call("");
// will fail because a fallback is found!
assertFalse(sent);
// will succeed because getSupply returns 0 when no horse is minted!
assertEq(uint(bytes32(data)), 0);
}

Recommended Mitigation

To address this issue, add a revert before labels in the MAIN macro.

#define macro MAIN() = takes (0) returns (0) {
.
.
.
dup1 __FUNC_SIG(ownerOf)eq ownerOf jumpi
​
+ 0x00 0x00 revert
​
totalSupply:
GET_TOTAL_SUPPLY()
.
.
.

This modification ensures that if an unknown function is called, the contract reverts, preventing the unintended execution of GET_TOTAL_SUPPLY and maintaining compliance with Solidity contracts.

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

MAIN() macro is not properly implemented

Any call data sent to the contract that doesn't contain a function selector will randomly mint a horse.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.