safe-global / safe-eth-py / 34131120785

93%

Build Type

push

github

Committed by web-flow

Commit Message

Validate contract signatures against the verifying Safe's EIP-1271 convention (#2726)

* Validate contract signatures against the verifying Safe's EIP-1271 convention

`SafeSignatureContract.is_valid` tried both EIP-1271 entrypoints and returned
True if either answered. On-chain only one is ever called, and the version of
the Safe doing the verifying decides which: below 1.5.0 `checkNSignatures`
calls the legacy `isValidSignature(bytes,bytes)` with the preimage, from 1.5.0
`checkContractSignature` calls `isValidSignature(bytes32,bytes)` with the hash,
and the 1.5.0 CompatibilityFallbackHandler no longer declares the legacy
overload at all. So a signature could be valid here and still revert GS024 on
the Safe that has to verify it.

Add `safe_version` to `is_valid` so only the entrypoint that version calls is
checked. It goes on the abstract `SafeSignature` / `SafeSignatureAsync` and on
every implementation, the same way `safe_address` already works: validation
context that only one signature type reads and the rest ignore.
`safe_version=None` keeps the old both-entrypoints behaviour, so existing
callers are unaffected.

Add `uses_bytes32_eip1271()` as the single home for the version rule and for
its failure mode. `VERSION()` is an arbitrary on-chain string, so it raises
`CannotCheckEIP1271ContractSignature` instead of leaking packaging's
`InvalidVersion` and making every consumer reimplement the translation.

* Require the magic value the verifying Safe expects for each entrypoint

`_check_eip1271` accepted either magic value whichever entrypoint was called. The
two interfaces declare different ones and the Safe compares against its own,
exactly: `ISignatureValidator.EIP1271_MAGIC_VALUE` is 0x20c13b0b up to 1.4.1 and
0x1626ba7e from 1.5.0.

Every Safe fallback handler returns the value matching the entrypoint, so no Safe
signer hits this. An owner can be any EIP-1271 contract though, and one that
answers the other interface's value was reported valid ... (continued)

Coverage Stats

137 of 139 new or added lines in 2 files covered. (98.56%)

1 existing line in 1 file now uncovered.

11304 of 12098 relevant lines covered (93.44%)

3.74 hits per line