mt030d - Incorrect encoding of bytes for EIP712 digest in TitleGraph causes signatures generated by common EIP712 tools to be unusable

[sherlock-admin3]

mt030d

medium

Incorrect encoding of bytes for EIP712 digest in TitleGraph causes signatures generated by common EIP712 tools to be unusable

Summary

The signature in TitleGraph.acknowledgeEdge() and TitleGraph.unacknowledgeEdge() is generated based on a digest computed from edgeId and data. However, the data bytes argument is not correctly encoded according to the EIP712 specification. Consequently, a signature generated using common EIP712 tools would not pass validation in TitleGraph.checkSignature().

Vulnerability Detail

According to EIP712:

The dynamic values bytes and string are encoded as a keccak256 hash of their contents.

    modifier checkSignature(bytes32 edgeId, bytes calldata data, bytes calldata signature) {
        bytes32 digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data)));
        ...
    }

However, the checkSignature() modifier in the TitlesGraph contract reconstructs the digest by encoding the data bytes argument without first applying keccak256 hashing.
As a result, a signature generated using common EIP712 tools (e.g. using the signTypedData function from ethers.js) would not pass validation in TitleGraph.checkSignature().

POC

1. EIP712 signature computed by using ethers.js

// main.js
const { ethers } = require("ethers");

async function main() {
    const pk = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80";
    const signer = new ethers.Wallet(pk);
    const domain = {
      name: "TitlesGraph",
      version: '1',
      chainId: 31337,
      verifyingContract: "0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496" // should match the address in foundry test
    };
    const types = {
      Ack: [
        { name: "edgeId", type: "bytes32" },
        { name: "data", type: "bytes" },
      ],
    };
    const value = {
        edgeId: ethers.id("test edgeId"),
        data: "0xabcd"
    };
    const signature = await signer.signTypedData(domain, types, value);
    console.log(signature);
    
}

main();

here we run

npm install ethers
node main.js

The output is 0xab4623a7bacf25ed3d6779684f195ed63a5ed1ed46c278c107390086e74b739b35f1db213c6075dedc041d68ced3d11798d49afaf3c47743d4696c49f03037b51b

2. EIP712 signature computed using foundry

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {EIP712} from "lib/solady/src/utils/EIP712.sol";

contract EIP712Test is Test, EIP712 {
    bytes32 public constant ACK_TYPEHASH = keccak256("Ack(bytes32 edgeId,bytes data)");

    // test data
    bytes32 testEdgeId = keccak256("test edgeId");
    bytes testData = hex"abcd";


    function test_sig() public {
        uint256 pk = 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80;
        bytes32 digest = _computeDigest(testEdgeId, testData);
        (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, digest);
        bytes memory signature = abi.encodePacked(r, s, v);
        console.logBytes(signature);
    }

    function test_sigShouldBe() public {
        uint256 pk = 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80;
        bytes32 digest = _computeDigestShouldBe(testEdgeId, testData);
        (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, digest);
        bytes memory signature = abi.encodePacked(r, s, v);
        console.logBytes(signature);
    }

    function _computeDigest(bytes32 edgeId, bytes memory data) internal returns (bytes32 digest) {
        digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data)));
    }

    function _computeDigestShouldBe(bytes32 edgeId, bytes memory data) internal returns (bytes32 digest) {
        digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, keccak256(data))));
    }

    function _domainNameAndVersion()
        internal
        pure
        override
        returns (string memory name, string memory version)
    {
        name = "TitlesGraph";
        version = "1";
    }
}

here we run

forge test --mc EIP712Test -vv

The output is

[PASS] test_sig() (gas: 12176)
Logs:
  0x7bd09aece710ef3845f26c4a695d357b1b170f75d0702f18ec09409f571260237a38e0fed802f8a9d598d9aed0d7898562c51e09bfa7cf254e5a8a5bc74106561c

[PASS] test_sigShouldBe() (gas: 11958)
Logs:
  0xab4623a7bacf25ed3d6779684f195ed63a5ed1ed46c278c107390086e74b739b35f1db213c6075dedc041d68ced3d11798d49afaf3c47743d4696c49f03037b51b

test_sig() simulates the way the digest is reconstructed in TitleGraph.checkSignature(), while test_sigShouldBe() shows how the digest should be reconstructed.
From the above output, we can see the signature generated by ethers.js matches the signature generated in test_sigShouldBe() and does not match the signature generated in test_sig().
This PoC shows the way TitleGraph.checkSignature() reconstruct the digest is not compatible with the way data is encoded in EIP712.

Impact

A signature generated by the signer using common EIP712 tools (e.g. signTypedData in ethers.js) would not pass validation in TitleGraph.checkSignature().

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L41

Tool used

Manual Review, ethers.js, foundry

Recommendation

Encoding the data bytes as a keccak256 hash of its contents before computing the digest from it:

- digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data)));
+ digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, keccak256(data))));

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
https://github.com/titlesnyc/wallflower-contract-v2/pull/1

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.