Concern regarding secret token validation and timing attacks

[quadratz]

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?

[KnorpelSenf]

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.

[KnorpelSenf]

ref https://jsr.io/@std/crypto/doc/~/timingSafeEqual