Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

HorseStore.huff::main() implementation of function selector matching is missing a revert

Summary

HorseStore.huff::main() implementation of function selector matching is missing a revert

Vulnerability Details

HorseStore.huff::main() implementation of function selector matching is missing a revert,When calling a non-existent function, due to the absence of revert, totalSupply() is executed when there is no matching function signature

Add this test to Base_Test.t.sol and run forge test --mt testCallNotExistFunc -vvvv to validate the issue

function testCallNotExistFunc() public payable {
vm.startPrank(user);
horseStore.mintHorse();
horseStore.mintHorse();
(bool success, bytes memory data) = address(horseStore).call{value: msg.value}(
abi.encodeWithSignature("foo(string,uint256)", "call foo", 123)
);
assertEq(horseStore.totalSupply(), abi.decode(data, (uint256)));
}

The output result confirms that when calling a non-existent function, it actually executes totalSupply().

[111319] HorseStoreHuff::testCallNotExistFunc()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [71041] 0x6d2eed85750d316088343D6d5e91ca59eb052768::mintHorse()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 0)
│ └─ ← ()
├─ [27241] 0x6d2eed85750d316088343D6d5e91ca59eb052768::mintHorse()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 1)
│ └─ ← ()
├─ [501] 0x6d2eed85750d316088343D6d5e91ca59eb052768::foo("call foo", 123)
│ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000002
├─ [149] 0x6d2eed85750d316088343D6d5e91ca59eb052768::totalSupply() [staticcall]
│ └─ ← 2
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 463.05ms

Impact

Any call to a non-existent function will return totalSupply.

Tools Used

manual inspection.

Recommendations

Add a revert in MAIN(); it will revert when there is no function match

#define macro MAIN() = takes (0) returns (0) {
// Identify which function is being called.
0x00 calldataload 0xE0 shr
dup1 __FUNC_SIG(totalSupply) eq totalSupply jumpi
dup1 __FUNC_SIG(feedHorse) eq feedHorse jumpi
dup1 __FUNC_SIG(isHappyHorse) eq isHappyHorse jumpi
dup1 __FUNC_SIG(horseIdToFedTimeStamp) eq horseIdToFedTimeStamp jumpi
dup1 __FUNC_SIG(mintHorse) eq mintHorse jumpi
dup1 __FUNC_SIG(HORSE_HAPPY_IF_FED_WITHIN) eq horseHappyIfFedWithin jumpi
dup1 __FUNC_SIG(approve) eq approve jumpi
dup1 __FUNC_SIG(setApprovalForAll) eq setApprovalForAll jumpi
dup1 __FUNC_SIG(transferFrom) eq transferFrom jumpi
dup1 __FUNC_SIG(name) eq name jumpi
dup1 __FUNC_SIG(symbol) eq symbol jumpi
dup1 __FUNC_SIG(tokenURI) eq tokenURI jumpi
dup1 __FUNC_SIG(supportsInterface)eq supportsInterface jumpi
dup1 __FUNC_SIG(getApproved) eq getApproved jumpi
dup1 __FUNC_SIG(isApprovedForAll) eq isApprovedForAll jumpi
dup1 __FUNC_SIG(balanceOf) eq balanceOf jumpi
dup1 __FUNC_SIG(ownerOf)eq ownerOf jumpi
+
+ 0x00 0x00 revert
+
totalSupply:
GET_TOTAL_SUPPLY()
Updates

Lead Judging Commences

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.