camo
camo copied to clipboard
Use a time based comparison
Use a constant time comparison rather than HMAC comparison
I like the sentiment of this change, but crypto.timingSafeEqual
throws a TypeError
if the two buffer lengths are not equal. Personally, I think this is a failure of the design of this function, because it allows for timing attacks to potentially be able to determine the exact length of a secret, but that's a tangential discussion.
> Crypto = require('crypto')
> Crypto.timingSafeEqual(Buffer.from("1234"), Buffer.from("1234"))
true
> Crypto.timingSafeEqual(Buffer.from("4321"), Buffer.from("1234"))
false
> Crypto.timingSafeEqual(Buffer.from("12345"), Buffer.from("1234"))
TypeError: Input buffers must have the same length
at repl:1:8
at ContextifyScript.Script.runInThisContext (vm.js:50:33)
at REPLServer.defaultEval (repl.js:240:29)
at bound (domain.js:301:14)
at REPLServer.runBound [as eval] (domain.js:314:12)
...
Should we catch this TypeError
to ensure a "four_oh_four
" response or just allow a 50x
error to bubble up?
I don't even know what that means
"four_oh_four
" would make more sense to me, otherwise we're potentially revealing information about the existence of an asset.