grammyjs
Concern regarding secret token validation and timing attacks
https://github.com/grammyjs/grammY/blob/48153f97840df5af0ce3706313d5052c370e0fce/src/convenience/webhook.ts#L112-L117
A timing attack is when an attacker can infer information about a secret by measuring how long it takes to compare values. If the comparison time varies based on the content, it could be exploited.
I found a relevant discussion on Stack Overflow that touches on this topic: https://stackoverflow.com/questions/31095905/whats-the-difference-between-a-secure-compare-and-a-simple
To address this risk, I was thinking we could implement a constant-time comparison. Here’s a rough idea (still untested):
const te = new TextEncoder();
const headerHash = await crypto.subtle.digest(
"SHA-256",
te.encode(header),
);
const tokenHash = await crypto.subtle.digest(
"SHA-256",
te.encode(token),
);
if (timingSafeEqual(headerHash, tokenHash)) {
await unauthorized();
// TODO: investigate deno bug that happens when this console logging is removed
console.log(handlerReturn);
return handlerReturn;
}
Does it make sense to be concerned about timing attacks in this context?
Yes, it makes sense to think about this. There was a good reason why this was not done from the start. Unfortunately, I do not recall it. If nobody can think of a reason why this is not needed, then we should have a constant-time string comparison here.
Sure thing, please go ahead. It might be challenging to do this for v1 but it's trivial for v2. Curious what you'll come up with.