Open c4-submissions opened 10 months ago
raymondfam marked the issue as high quality report
raymondfam marked the issue as primary issue
0xad1onchain (sponsor) confirmed
The Warden has demonstrated that the contract is non-compliant with EIP-712 as the type-hashes defined within it are outdated and incorrect.
The Warden has articulated what the potential impact is and, while slightly brief, the recommendation the Warden provided is correct.
As a result, I fully accept this submission as satisfactory as well as the best of its kind. #182 will be awarded partial credit given that it managed to pinpoint a single flaw in a single type-hash rather than identify that multiple type hashes were incorrect.
alex-ppg marked the issue as satisfactory
alex-ppg marked the issue as selected for report
Fixed, amended EIP 712 struct
Lines of code
https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/libraries/TypeHashHelper.sol#L45-L50 https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/libraries/TypeHashHelper.sol#L52-L57
Vulnerability details
Summary
When implementing
EIP712
, among other things, for data structures that will be a part of signing message a typehash must be defined.The structure typehash is defined as:
typeHash = keccak256(encodeType(typeOf(s)))
name ‖ "(" ‖ member₁ ‖ "," ‖ member₂ ‖ "," ‖ … ‖ memberₙ ")"
type ‖ " " ‖ name
.The project uses 2 structures on the signed data
Transaction
andValidation
declared inTypeHashHelper
:However, the precalculated typehash for each of the structure are of different structures:
Transaction
the hash is actually calculated on a now removedExecutionParams
structure:Validation
the hash is calculated on another old, removed structureValidationParams
:POC
The issue went undetected in the initial development phase, and can be verified, because the tests still use the old hashes:
The specific test can be verified by running:
Impact
Protocol is not EIP712 compliant which will result in issues with integrators.
Tools Used
Manual review and an online
keccak256
checker for validating that the those hashes are not actually for the correct structures.Recommendations
Modify the structure typehash (and tests) to point to the correct structures.
Assessed type
Other