Submission Details
Severity:
The declaration and use of `LibTractor::BLUEPRINT_TYPE_HASH` are inconsistent with the structure `struct Blueprint`, and the standard is confusing. It is recommended to unify the standard
Summary
The declaration and use of LibTractor::BLUEPRINT_TYPE_HASH is inconsistent with the field name of the structure struct Blueprint. This inconsistency may cause signature verification to fail.
Vulnerability Details
bytes32 private constant TRACTOR_HASHED_NAME = keccak256(bytes("Tractor"));
bytes32 private constant TRACTOR_HASHED_VERSION = keccak256(bytes("1"));
bytes32 private constant EIP712_TYPE_HASH =
keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
);
bytes32 public constant BLUEPRINT_TYPE_HASH =
keccak256(
@> "Blueprint(address publisher,bytes data,bytes operatorData,uint256 maxNonce,uint256 startTime,uint256 endTime)"
);
struct TractorStorage {
// Number of times the blueprint has been run.
mapping(bytes32 => uint256) blueprintNonce;
// Publisher Address => counter id => counter value.
mapping(address => mapping(bytes32 => uint256)) blueprintCounters;
// Publisher of current operations. Set to address(1) when no active publisher.
address payable activePublisher;
}
// Blueprint stores blueprint related values
struct Blueprint {
address publisher;
bytes data;
@> bytes32[] operatorPasteInstrs;
uint256 maxNonce;
uint256 startTime;
uint256 endTime;
}
And MessageHashUtils.toEthSignedMessageHash() in TractorFacet::verifyRequisition() adds a fixed prefix, which should be EIP-191, which is different from the EIP-712 standard.
modifier verifyRequisition(LibTractor.Requisition calldata requisition) {
bytes32 blueprintHash = LibTractor._getBlueprintHash(requisition.blueprint);
require(blueprintHash == requisition.blueprintHash, "TractorFacet: invalid hash");
address signer = ECDSA.recover(
@> MessageHashUtils.toEthSignedMessageHash(requisition.blueprintHash),
requisition.signature
);
require(signer == requisition.blueprint.publisher, "TractorFacet: signer mismatch");
_;
}
The test was performed correctly because the standard Ethereum message signing method (EIP-191) was used in the test, but problems would occur if it was replaced with EIP-712 (see the Poc below).
// EIP-191
signature = await signer.signMessage(
ethers.utils.arrayify(blueprintHash)
);
// EIP-712
signature = await signer._signTypedData(domain, types, blueprint);
Poc
// EIP712Test.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
contract EIP712Test is EIP712 {
string private constant SIGNING_DOMAIN = "EIP712Test";
string private constant SIGNATURE_VERSION = "1";
// Blueprint struct definition
struct Blueprint {
address publisher;
bytes data;
bytes32[] operatorPasteInstrs;
uint256 maxNonce;
uint256 startTime;
uint256 endTime;
}
// Correct type hash definition
bytes32 public constant BLUEPRINT_TYPE_HASH_CORRECT =
keccak256(
"Blueprint(address publisher,bytes data,bytes32[] operatorPasteInstrs,uint256 maxNonce,uint256 startTime,uint256 endTime)"
);
// Incorrect type hash definition
bytes32 public constant BLUEPRINT_TYPE_HASH_INCORRECT =
keccak256(
"Blueprint(address publisher,bytes data,bytes operatorData,uint256 maxNonce,uint256 startTime,uint256 endTime)"
);
constructor() EIP712(SIGNING_DOMAIN, SIGNATURE_VERSION) {}
// Function to hash the Blueprint struct with the correct type hash
function getBlueprintHashCorrect(Blueprint calldata blueprint) public view returns (bytes32) {
return
_hashTypedDataV4(
keccak256(
abi.encode(
BLUEPRINT_TYPE_HASH_CORRECT,
blueprint.publisher,
keccak256(blueprint.data),
keccak256(abi.encodePacked(blueprint.operatorPasteInstrs)),
blueprint.maxNonce,
blueprint.startTime,
blueprint.endTime
)
)
);
}
// Function to hash the Blueprint struct with the incorrect type hash
function getBlueprintHashIncorrect(Blueprint calldata blueprint) public view returns (bytes32) {
return
_hashTypedDataV4(
keccak256(
abi.encode(
BLUEPRINT_TYPE_HASH_INCORRECT,
blueprint.publisher,
keccak256(blueprint.data),
keccak256(abi.encodePacked(blueprint.operatorPasteInstrs)),
blueprint.maxNonce,
blueprint.startTime,
blueprint.endTime
)
)
);
}
// Function to verify the signature with the correct type hash
function verifySignatureCorrect(
Blueprint calldata blueprint,
bytes calldata signature
) public view returns (address) {
bytes32 digest = getBlueprintHashCorrect(blueprint);
return ECDSA.recover(digest, signature);
}
// Function to verify the signature with the incorrect type hash
function verifySignatureIncorrect(
Blueprint calldata blueprint,
bytes calldata signature
) public view returns (address) {
bytes32 digest = getBlueprintHashIncorrect(blueprint);
return ECDSA.recover(digest, signature);
}
}
// eip.test.js
const { ethers } = require("hardhat");
const { expect } = require("chai");
describe("EIP712Test", function () {
let EIP712Test;
let eip712Test;
let signer;
let publisher;
let blueprint;
let signature;
beforeEach(async function () {
[signer, publisher] = await ethers.getSigners();
EIP712Test = await ethers.getContractFactory("EIP712Test");
eip712Test = await EIP712Test.deploy();
await eip712Test.deployed();
blueprint = {
publisher: publisher.address,
data: ethers.utils.hexlify(ethers.utils.toUtf8Bytes("test data")),
operatorPasteInstrs: [ethers.utils.formatBytes32String("instr1")],
maxNonce: 1,
startTime: Math.floor(Date.now() / 1000),
endTime: Math.floor(Date.now() / 1000) + 3600
};
const domain = {
name: "EIP712Test",
version: "1",
chainId: (await ethers.provider.getNetwork()).chainId,
verifyingContract: eip712Test.address
};
const types = {
Blueprint: [
{ name: "publisher", type: "address" },
{ name: "data", type: "bytes" },
{ name: "operatorPasteInstrs", type: "bytes32[]" },
{ name: "maxNonce", type: "uint256" },
{ name: "startTime", type: "uint256" },
{ name: "endTime", type: "uint256" }
]
};
signature = await signer._signTypedData(domain, types, blueprint);
console.log("signer address:", signer.address);
});
it("should verify signature with correct type hash", async function () {
const recoveredAddress = await eip712Test.verifySignatureCorrect(blueprint, signature);
expect(recoveredAddress).to.equal(signer.address);
console.log("BLUEPRINT_TYPE_HASH_CORRECT:", recoveredAddress);
});
it("should not verify signature with incorrect type hash", async function () {
const recoveredAddress = await eip712Test.verifySignatureIncorrect(blueprint, signature);
expect(recoveredAddress).to.not.equal(signer.address);
console.log("BLUEPRINT_TYPE_HASH_INCORRECT:", recoveredAddress);
});
});
run test
EIP712Test
signer address: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
BLUEPRINT_TYPE_HASH_CORRECT: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
✔ should verify signature with correct type hash (429ms)
signer address: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
BLUEPRINT_TYPE_HASH_INCORRECT: 0x78b9981c5e24E92ED1c6dC386541bFB9a2C4048A
✔ should not verify signature with incorrect type hash
Code Snippet
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/libraries/LibTractor.sol#L22-L49
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/farm/TractorFacet.sol#L32-L41
Impact
The declaration and use of LibTractor::BLUEPRINT_TYPE_HASH is inconsistent with the field name of the structure struct Blueprint. This inconsistency may cause signature verification to fail. In addition, the standards are confusing. It is recommended to unify the standards.
Tools Used
Manual Review
Recommendations
Unified Signature Standard and modify BLUEPRINT_TYPE_HASH
bytes32 public constant BLUEPRINT_TYPE_HASH =
keccak256(
- "Blueprint(address publisher,bytes data,bytes operatorData,uint256 maxNonce,uint256 startTime,uint256 endTime)"
+ "Blueprint(address publisher,bytes data,bytes32[] operatorPasteInstrs,uint256 maxNonce,uint256 startTime,uint256 endTime)"
);
Updates
Lead Judging Commences
inallhonesty 12 months ago
Submission Judgement Published Assigned finding tags:
The declaration and use of `LibTractor::BLUEPRINT_TYPE_HASH` is inconsistent with the field name of the structure `struct Blueprint`
Appeal created
deadrosesxyz
12 months ago
inallhonesty
11 months ago
inallhonesty 11 months ago
Submission Judgement Published Assigned finding tags:
The declaration and use of `LibTractor::BLUEPRINT_TYPE_HASH` is inconsistent with the field name of the structure `struct Blueprint`
Prize pool breakdown
Total prize
250,000 USDC
nSLOC
12,28520 USDC / LOC
220,000 USDC
10,000 USDC
20,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.