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

Incorrect `isHappyHorse` Behavior for fresh mint or Non-Existent Horses in Huff Implementation

0x4non

Summary

The HorseStore's Huff implementation exhibits a critical logical flaw in the isHappyHorse function. Contrary to the expected behavior, as defined in the Solidity version, this function incorrectly returns true for never feeded or non-existent horse IDs, leading to a significant discrepancy and potential misinterpretation of the contract's state.

Vulnerability Details

In the Huff rendition of the HorseStore contract, the isHappyHorse(uint256 horseId) function fails to accurately validate the existence of a horse before determining its happiness state. Consequently, when queried for non-existent horses, the function returns true, implying these horses are happy. This behavior deviates from the Solidity implementation, where non-existent horses correctly return false for the same query.

Impact

This inconsistency can result in several issues:

  1. Misrepresentation of State: Users and external contracts interacting with the Huff-based contract may be misled into believing that non-existent horses are in a happy state.

  2. Logical Integrity: The discrepancy undermines the logical integrity and expected behavior of the contract, potentially affecting integrations and user interactions.

  3. Data Reliability: The reliability of data returned by the contract is compromised, affecting decision-making processes based on the contract's output.

Tools Used

Manual review

Recommendations

To address this flaw, the Huff implementation of isHappyHorse should be modified to include a check that verifies the existence of the horse ID before assessing its happiness. This can be aligned with the Solidity version, where the existence check is inherently managed by the ERC721Enumerable structure. A possible approach in Huff would be to implement a logic that compares the queried horse ID against the total number of minted horses (similar to checking against totalSupply() in Solidity) before proceeding with the happiness evaluation.

After implementing these changes, it is crucial to conduct comprehensive testing to ensure that the revised function behaves as expected without introducing new issues. Additionally, reviewing the overall contract design for similar discrepancies between the Solidity and Huff implementations would be prudent to maintain consistency and reliability.

POC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {Test, console2} from "forge-std/Test.sol";
import {HuffDeployer} from "foundry-huff/HuffDeployer.sol";
import {HorseStore} from "../src/HorseStore.sol";
​
import {IERC721Enumerable} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Enumerable.sol";
​
contract MixedHuffTest is Test {
string public constant NFT_NAME = "HorseStore";
string public constant NFT_SYMBOL = "HS";
string public constant horseStoreLocation = "HorseStore";
HorseStore horseStoreHuff;
HorseStore horseStoreSol;
​
function setUp() public {
horseStoreSol = new HorseStore();
horseStoreHuff = HorseStore(
HuffDeployer.config().with_args(bytes.concat(abi.encode(NFT_NAME), abi.encode(NFT_SYMBOL))).deploy(
horseStoreLocation
)
);
}
​
function testHappy() public {
address user = makeAddr("user");
vm.warp(2 days);
assertEq(horseStoreSol.isHappyHorse(80), horseStoreHuff.isHappyHorse(80));
​
// what about a freshly minted horse?
vm.prank(user);
horseStoreSol.mintHorse();
horseStoreHuff.mintHorse();
​
assertEq(horseStoreSol.isHappyHorse(0), horseStoreHuff.isHappyHorse(0));
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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