Snowman Merkle Airdrop
AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Typo in MESSAGE_TYPEHASH causes unexpected bytes object

datadata

Root + Impact

Description

  • In the contract SnowmanAirdrop the normal behavior for MESSAGE_TYPEHASH is that it be the hashed type signature of struct SnowmanClaim which should be the results of the string "SnowmanClaim(address receiver, uint256 amount)" being passed to the function keccak256(). Instead of address, the code has the typo addres.

  • The problem is that there is a typo (addres) in the code's string passed to keccak256(), and so whenever MESSAGE_TYPEHASH is used, it will return a bytes object that differs from what is expected. MESSAGE_TYPEHASH is used in the function getMessageHash(). getMessageHash() is used in the function claimSnowman() .

@> bytes32 private constant MESSAGE_TYPEHASH = keccak256("SnowmanClaim(addres receiver, uint256 amount)");
​
function getMessageHash(address receiver) public view returns (bytes32) {
if (i_snow.balanceOf(receiver) == 0) {
revert SA__ZeroAmount();
}
​
uint256 amount = i_snow.balanceOf(receiver);
​
return _hashTypedDataV4(
@> keccak256(abi.encode(MESSAGE_TYPEHASH, SnowmanClaim({receiver: receiver, amount: amount})))
);
}
​
function claimSnowman(address receiver, bytes32[] calldata merkleProof, uint8 v, bytes32 r, bytes32 s)
external
nonReentrant
{
. . .
@> if (!_isValidSignature(receiver, getMessageHash(receiver), v, r, s)) {
revert SA__InvalidSignature();
}
. . .
}

Risk

Likelihood:

  • The definition of MESSAGE_TYPEHASH will be incorrect as soon as the contract is deployed.

  • This will cause an unexpected bytes object to be returned by the function getMessageHash(), which is called when claimSnowman() is called.

Impact:

  • The typo will result in MESSAGE_TYPEHASH having an unexpected value.

  • The function getMessageHash() will use the unexpected value of MESSAGE_TYPEHASH in its construction of the message, and thus return an unexpected value itself.

  • With getMessageHash() returning an unexpected value, the veracity of the if-statement within the function claimSnowman() will now be thrown into question. However, the function _isValidSignature() doesn't seem to catch that an incorrect message typehash is being used. Why isn't it catching this?

Proof of Concept

// Here are our additions to TestSnowmanAirdrop.t.sol.
​
// additional import
import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
​
// Contract SnowmanAirdrop inherits EIP712 to use its function _hashTypedDataV4.
contract TestSnowmanAirdrop is Test, EIP712 {
constructor() EIP712("EIP712Test", "1") {}
​
/**
* @dev This function asserts that MESSAGE_HASH differs from what is expected.
* This function uses a getter function in airdrop to get MESSAGE_HASH.
*/
function testMessageHashDiffersFromExpectedHash() public view {
// ARRANGE
bytes32 currentMessageTypeHash = airdrop.getMessageTypeHash();
// bytes object from properly spelled hashed signature
bytes32 properMessageTypeHash = keccak256(
"SnowmanClaim(address receiver, uint256 amount)"
);
// To rule out scope, we recreate the hash with the typo in this scope
bytes32 thisScopeCurrentMessageTypeHash = keccak256(
"SnowmanClaim(addres receiver, uint256 amount)"
);
​
// ASSERT
// Assert that the thisScope version is the same as the getter version
assertEq(currentMessageTypeHash, thisScopeCurrentMessageTypeHash);
​
// Assert that the current MESSAGE_HASH is not equal to the expected, proper hash
assertNotEq(currentMessageTypeHash, properMessageTypeHash);
}
​
/**
* @dev This function asserts that the current value of MESSAGE_HASH will result
* in unexpected digests being constructed by getMessageHash().
*/
function testGetMessageHashReturnsUnexpectedHash() public view {
// ARRANGE / ACT
// current MESSAGE_TYPEHASH using getMessageTypeHash
bytes32 currentMessageTypeHash = airdrop.getMessageTypeHash();
// the expected value of MESSAGE_TYPEHASH from the properly spelled signature
bytes32 properMessageTypeHash = keccak256(
"SnowmanClaim(address receiver, uint256 amount)"
);
​
// We use adress alice
bytes32 hashedCurrentMessageTypeHash = airdrop.getMessageHash(alice);
uint256 aliceAmount = snow.balanceOf(alice);
​
// We inherit EIP712 to use _hashTypedDataV4()
bytes32 hashedProperMessageTypeHash = _hashTypedDataV4(
keccak256(
abi.encode(
properMessageTypeHash,
SnowmanAirdrop.SnowmanClaim({
receiver: alice,
amount: aliceAmount
})
)
)
);
​
// ASSERT
assertNotEq(hashedCurrentMessageTypeHash, hashedProperMessageTypeHash);
}
​
// TODO: FUNCTION TO EXTRACT data from keccak256(abi.encode(),
// esp. the MESSAGE_TYPEHASH in abi.encode

The tests above rely on an added getter function in the contract SnowmanAirdrop. This getter should be removed after testing.

function getMessageTypeHash() external pure returns (bytes32) {
return MESSAGE_TYPEHASH;
}

Recommended Mitigation

- bytes32 private constant MESSAGE_TYPEHASH = keccak256("SnowmanClaim(addres receiver, uint256 amount)");
+ bytes32 private constant MESSAGE_TYPEHASH = keccak256("SnowmanClaim(address receiver, uint256 amount)");

By updating addres to address, we correctly pass the type signature of event SnowmanClaim to keccak256() for the definition of MESSAGE_TYPEHASH. With our expected type signature now hashed, we should not get errors or an unexpected message hash when we call getMessageHash().

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-02] Unconsistent `MESSAGE_TYPEHASH` with standart EIP-712 declaration on contract `SnowmanAirdrop`

# Root + Impact ## Description * Little typo on `MESSAGE_TYPEHASH` Declaration on `SnowmanAirdrop` contract ```Solidity // src/SnowmanAirdrop.sol 49: bytes32 private constant MESSAGE_TYPEHASH = keccak256("SnowmanClaim(addres receiver, uint256 amount)"); ``` **Impact**: * `function claimSnowman` never be `TRUE` condition ## Proof of Concept Applying this function at the end of /test/TestSnowmanAirdrop.t.sol to know what the correct and wrong digest output HASH. Ran with command: `forge test --match-test testFrontendSignatureVerification -vvvv` ```Solidity function testFrontendSignatureVerification() public { // Setup Alice for the test vm.startPrank(alice); snow.approve(address(airdrop), 1); vm.stopPrank(); // Simulate frontend using the correct format bytes32 FRONTEND_MESSAGE_TYPEHASH = keccak256("SnowmanClaim(address receiver, uint256 amount)"); // Domain separator used by frontend (per EIP-712) bytes32 DOMAIN_SEPARATOR = keccak256( abi.encode( keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), keccak256("Snowman Airdrop"), keccak256("1"), block.chainid, address(airdrop) ) ); // Get Alice's token amount uint256 amount = snow.balanceOf(alice); // Frontend creates hash using the correct format bytes32 structHash = keccak256( abi.encode( FRONTEND_MESSAGE_TYPEHASH, alice, amount ) ); // Frontend creates the final digest (per EIP-712) bytes32 frontendDigest = keccak256( abi.encodePacked( "\x19\x01", DOMAIN_SEPARATOR, structHash ) ); // Alice signs the digest created by the frontend (uint8 v, bytes32 r, bytes32 s) = vm.sign(alKey, frontendDigest); // Digest created by the contract (with typo) bytes32 contractDigest = airdrop.getMessageHash(alice); // Display both digests for comparison console2.log("Frontend Digest (correct format):"); console2.logBytes32(frontendDigest); console2.log("Contract Digest (with typo):"); console2.logBytes32(contractDigest); // Compare the digests - they should differ due to the typo assertFalse( frontendDigest == contractDigest, "Digests should differ due to typo in MESSAGE_TYPEHASH" ); // Attempt to claim with the signature - should fail vm.prank(satoshi); vm.expectRevert(SnowmanAirdrop.SA__InvalidSignature.selector); airdrop.claimSnowman(alice, AL_PROOF, v, r, s); assertEq(nft.balanceOf(alice), 0); } ``` ## Recommended Mitigation on contract `SnowmanAirdrop` Line 49 applying this: ```diff - bytes32 private constant MESSAGE_TYPEHASH = keccak256("SnowmanClaim(addres receiver, uint256 amount)"); + bytes32 private constant MESSAGE_TYPEHASH = keccak256("SnowmanClaim(address receiver, uint256 amount)"); ```

Support

FAQs

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

Give us feedback!