Gnosis Safe's contracts take a FINAL, VALID EIP-712 signed hash that was specifically created in accordance with the EIP-712 specification and then NESTS THAT within the EIP-191 signature scheme for creating data hashes to verify against signatures for PERSONAL SIGNED MESSAGES for verifying TRANSACTIONS.
Multitude of Problems + Vulnerabilities That Spawn From Doing This
From the jump, this incorrect and flawed implementation of EIP-191: (a) destroys domain separation for all EIP-712 messages submitted to Safe (b) destroys replay protection / nonce effectiveness entirely and (c) allows data that was signed and intended to be used in one context (i.e., a signed transaction) to also be validated in another (i.e., as a personal signed message). (d) Most importantly, this implementation means we have no clue what signatures are being verified against if the
v byte is > 30. This is due to the fact that this condition means the validly constructed EIP-712 hash of the encoded transaction data (which included the SAFE_TX_TYPEHASH value and domain separator directly in the hash of transaction data & encoded transaction information, respectively), has its cryptographic uniqueness properties destroyed whenever the value of v is determined to be > 30, because that results in the EVM taking that EIP-712 signed hash and prepending it with the 0x1945... EIP-191 prefix (ASCII for \x19 Ethereum Signed Message\n32`), concatenates the two values then performs a keccak256 hash on that concatenated result. (This is nowhere near or close to compliant with EIP-191)What that literally translates to is:
keccak256(EIP-191 prefix + EIP-712 signed hash of encoded TX data) = EIP-191 personal_sign hash. The EIP-712 hash is still provided in the event logs of the transaction because the checkSignatures routine (and the calculations that result in the derivation of the non-compliant, vulnerable EIP-191 signed message scheme) are all called, defined and relevant within a scoped block within the execTransaction function. A scoped block's definitions and declarations are only valid until that block of code (sort of like a sub-function within a function) have finished executing.Why This is a Serious, Critical Issue
This is a significant flaw because EIP-191 is expecting to receive personal sign data (messages) that gets interpreted as OPAQUE BYTES whereas EIP-712 is expecting STRUCTURED TRANSACTION DATA THAT GETS INTERPRETED AND HASHED IN A CERTAIN WAY. Arbitrarily choosing to interpret data validly presented as a transaction (signed_typed_v3 / signed_typed_v4) as a personal sign message instead results in wildly unpredictable results as it pertains to signature validation.
Thus, it is very possible the owners of the Bybit proxy never signed ANY malicious (`execTransaction`) transaction explicitly (javascript injection notwithstanding) (contrary to the 'blind signing' narrative; we can state this definitively, since the signatures for that hack transaction all signed transaction data that was at least presented to them in the form of a structured transaction and processed like one in accordance with the
EIP-712 standard). The only code in the implementation/masterContract/singleton source code stipulating this alternative signature validation pathway for EIP-191 "compliant" data ('compliant' in quotes here since the Safe implementation of this signature standard isn't even in that universe) is contingent on the the v byte for each individual, provided signature value of the signatures [why let signers dictate this? this essentially allows the contract caller to choose their preferred means of validation via simple bit manipulation]).