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

Huff Implementation Not Rejecting Ether Transfers

0x4non

Summary

The Horse Store project's Huff rendition of its NFT contract currently lacks the functionality to reject Ether transfers. This omission potentially allows users to inadvertently send Ether to the contract, which may result in the loss of funds, as the contract does not have a mechanism to return or utilize these Ether.

Vulnerability Details

In its current state, the Huff version of the NFT contract does not contain checks or restrictions against receiving Ether. This could happen in two ways:

  1. Direct Ether transfers to the contract address.

  2. Executing functions with a non-zero msg.value.

Without appropriate safeguards, these actions could lead to Ether being permanently locked within the contract, as there is no function to withdraw or refund the Ether.

Impact

The potential impact of this vulnerability includes:

  • Unintentional loss of funds for users who mistakenly send Ether to the contract.

  • Negative user experience and possible reputational damage for the project.

  • Increased scrutiny and potential security concerns from the community and stakeholders.

Tools Used

Manual review

Recommendations

To mitigate this vulnerability and enhance the security of the contract, is recommended to implement a check in the Huff dispatcher to reject all incoming Ether transfers. This can be achieved by adding a condition that reverts the transaction if msg.value is greater than zero.

diff --git a/src/HorseStore.huff b/src/HorseStore.huff
index 4e17fea..e72ac9d 100644
--- a/src/HorseStore.huff
+++ b/src/HorseStore.huff
@@ -1,6 +1,7 @@
/* Imports */
#include "../lib/huffmate/src/data-structures/Hashmap.huff"
#include "../lib/huffmate/src/utils/CommonErrors.huff"
+#include "../lib/huffmate/src/auth/NonPayable.huff"
/* HorseStore Interface */
#define function mintHorse() nonpayable returns ()
@@ -142,6 +143,9 @@
}
#define macro MAIN() = takes (0) returns (0) {
+ // reject any call with value
+ NON_PAYABLE()
+
// Identify which function is being called.
0x00 calldataload 0xE0 shr

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 testRejectEther() public {
vm.expectRevert();
payable(address(horseStoreSol)).transfer(1 ether);
​
(bool success, ) = address(horseStoreSol).call{value: 1 ether}("");
assertFalse(success, "should fail");
​
// lets try with huff version
vm.expectRevert();
payable(address(horseStoreHuff)).transfer(1 ether);
​
(success, ) = address(horseStoreHuff).call{value: 1 ether}("");
assertFalse(success, "should fail");
}
​
​
function testMintWithValue() public {
address user = makeAddr("user");
deal(user, 1 ether);
vm.warp(10);
vm.roll(10);
vm.startPrank(user);
(bool success, ) = address(horseStoreSol).call{value: 1 ether}(abi.encodeWithSignature("mintHorse()"));
assertTrue(!success, "mintHorse() should fail");
// lets try the same with huff version
​
/// sending ether should revert
(success, ) = address(horseStoreHuff).call{value: 1 ether}(abi.encodeWithSignature("mintHorse()"));
assertTrue(!success, "mintHorse() should fail");
​
vm.stopPrank();
}
}
Updates

Lead Judging Commences

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

HUFF functions are payable when they shouldn't be

Support

FAQs

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