Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check Stripe-Signature timestamp to prevent some replay "attacks" #129

Open
exarkun opened this issue Oct 28, 2022 · 3 comments
Open

Check Stripe-Signature timestamp to prevent some replay "attacks" #129

exarkun opened this issue Oct 28, 2022 · 3 comments

Comments

@exarkun
Copy link
Collaborator

exarkun commented Oct 28, 2022

This is not a security issue.

The value of the Stripe-Signature header field includes a timestamp which is meant to indicate the time at which the request carrying the signature was made. The timestamp is cryptographically protected by the signature. Stripe recommends rejecting requests which carry a timestamp that is too old (they say the libraries they distribute reject timestamps older than 5 minutes).

In 240.stripe-webhook-and-payment-intents we are adding a webhook which consumes one event - checkout.session.completed - and interprets it to mean a voucher should be marked as paid. Vouchers are single use and the only three states they can exist in are "not paid", "paid and unredeemed", and "paid and redeemed". The Stripe webhook uses an internal API which can only change the state from "not paid" to "paid and unredeemed". In either of the other states, the internal API fails with an error (which we will propagate to the response to the webhook request). Thus, a replay attack cannot extract any additional value from the system. At most it can make us do a little more database work than might otherwise be necessary.

Still, in the future maybe something about the situation will change and preventing replays will be useful. So, implement general timestamp checking in the Stripe signatures and refuse to process requests that come with a signature that is "too old".

@exarkun
Copy link
Collaborator Author

exarkun commented Nov 2, 2022

Here's a suggestion for an example of how to do a timing-attack-resistant comparison - dminuoso/happstack-server@02acdf7

@meejah
Copy link
Collaborator

meejah commented Nov 2, 2022

This ticket is about checking timestamps though, right?

(Not constant-time comparison? As the comments in there suggest, something based on hashing is a common approach ... which would also leak the length ... but they're sending us a signature or HMAC right?)

@exarkun
Copy link
Collaborator Author

exarkun commented Nov 2, 2022

Uhh yea, was just trying to clear out my browser tabs and didn't think hard enough.

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

No branches or pull requests

2 participants