Skip to content

fix(web3): honor SIWE ExpirationTime when NotBefore is absent#2488

Open
vatsalpatel wants to merge 1 commit intosupabase:masterfrom
vatsalpatel:fix/siwe-expiration-without-notbefore
Open

fix(web3): honor SIWE ExpirationTime when NotBefore is absent#2488
vatsalpatel wants to merge 1 commit intosupabase:masterfrom
vatsalpatel:fix/siwe-expiration-without-notbefore

Conversation

@vatsalpatel
Copy link
Copy Markdown

What kind of change does this PR introduce?

Bug fix; Closes #2453.

What is the current behavior?

In web3GrantEthereum (internal/api/web3.go), the SIWE ExpirationTime check is gated on NotBefore != nil:

if parsedMessage.NotBefore != nil && parsedMessage.ExpirationTime != nil && !parsedMessage.ExpirationTime.IsZero() && now.After(*parsedMessage.ExpirationTime) {

Per EIP-4361, not-before and expiration-time are independent optional fields. A SIWE message that sets expirationTime but omits notBefore causes the entire expiration check to be skipped, and an expired message is accepted.

The Solana handler checks the two fields independently and is unaffected.

MaximumValidityDuration does not compensate: it bounds validity based on IssuedAt, not the per-message ExpirationTime. A message with a 5-minute expiry is still accepted for the full MaximumValidityDuration window.

What is the new behavior?

Drop the parsedMessage.NotBefore != nil && clause so ExpirationTime is honored whenever it is set, independent of NotBefore. The adjacent NotBefore check is already independent and is untouched.

Test coverage is filled out across the NotBefore / ExpirationTime matrix so future regressions are caught:

NotBefore ExpirationTime Expected Test
nil nil accept TestHappyPath_MinimalMessage (existing)
nil nil (too old) reject TestValidationRules_IssedTooLongAgo (new case)
nil past reject — the regression TestValidationRules_Expired (new case)
nil future accept TestHappyPath_FullMessage (new case)
future nil reject (NotBefore) TestValidationRules_ValidatedBeforeNotBefore (existing)
past past reject (ExpirationTime) TestValidationRules_Expired (existing)
past future accept TestHappyPath_FullMessage (existing)
future future reject (NotBefore fires first) TestValidationRules_ValidatedBeforeNotBefore (new case)
NotBefore > ExpirationTime parser rejects (ErrNotBeforeAfterExpiration) internal/utilities/siwe/parser_test.go (existing)

Additional context

  • !parsedMessage.ExpirationTime.IsZero() is kept alongside the nil check. The asymmetry with the Solana handler (value type, not pointer) is intrinsic to the SIWE parser types, not something to normalize.
  • Signatures for the four new Ethereum test cases were generated offline with a single deterministic throwaway ECDSA key (0x2638aB94…) to keep the diff tight.

Per EIP-4361, NotBefore and ExpirationTime are independent optional fields. The Ethereum handler gated the expiration check on NotBefore != nil, silently accepting expired messages that omitted NotBefore. MaximumValidityDuration does not compensate — a short per-message expiry was skipped entirely.

Fills out test matrix coverage: NotBefore-absent + ExpirationTime-past (regression), NotBefore-absent + ExpirationTime-future (accept), both absent + too old (reject), both in future (NotBefore fires first).

Closes supabase#2453
@vatsalpatel vatsalpatel requested a review from a team as a code owner April 20, 2026 05:49
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.

SIWE ExpirationTime check skipped when NotBefore is absent

1 participant