Skip to content

Conversation

@ScreamingHawk
Copy link
Collaborator

This was raised during the C4 audit.

S-157 BaseAuth.recoverSapientSignature returns a constant instead of signer image hash, breaking sapient signer flows

Issue: Wallets always return bytes32(1) as their imageHash when responding to recoverSapientSignature. This goes against the ISapient interface expectation.

Analysis: The decision to return bytes32(1) as the wallet image hash was intentional. Otherwise, a parent wallet using a child wallet will have trouble using them since we expect the image hash changes. However, it may be desirable for a parent to only support a child wallet with a fixed configuration.

Fix: The adds a new signature flag byte is prepended when using recoverSapientSignature. This has a bool on whether to return the recovered image hash or bytes32(1), allowing a parent to support both changing and static image hashes. When the flag is not supplied (i.e. we are returning bytes32(1)), we ban the use of chained signatures to prevent unpersisted image hash updates returning the currently accepted image hash.

Caveat: When using a static signature, the image hash cannot always be recovered. In this case we return bytes32(bytes1(0x80)) (the static signature flag) as the image hash.

Enhancement: Update the ISapient spec to define bytes(1) (or similar) as a constant that represents "any image hash", so that other ISapient integrators can follow the same pattern.

Alternative: Remove the new sapient signature flag byte, always return the derived image hash. A parent wallet should use the child wallet as a signer under the 1271 interface when supporting dynamic image hashes. The caveat with this approach is that we lose the ability for the child to validate the payload contents without the ISapient interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants