From 94f270857dc4532416f540729692f7e81f5bcfcf Mon Sep 17 00:00:00 2001 From: Christian Bundy Date: Fri, 4 Sep 2020 08:38:03 -0700 Subject: [PATCH] Fix verification using bitwise OR on booleans Problem: Using bitwise operations on booleans doesn't make any sense, and I didn't even catch that this was a problem until I ran `tsc` on the codebase and saw the complaint. Static analysis rocks. In the libsodium code these methods return `0` or `-1`, so the bitwise OR acts like a boolean AND (`(0 | -1) === -1` like `(true && false === false)`. Solution: Convert bitwise OR to boolean AND and then confirm that this is the reason that the faulty truncated comparison fixed in fa39bc5 is now captured by the tests in my pull request into sodium-test. See-Also: https://github.com/sodium-friends/sodium-test/pull/14 --- crypto_auth.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crypto_auth.js b/crypto_auth.js index 4c5e463..fb3d3e6 100644 --- a/crypto_auth.js +++ b/crypto_auth.js @@ -26,7 +26,7 @@ function crypto_auth_hmacsha256 (out, input, k) { function crypto_auth_hmacsha256_verify (h, input, k) { const correct = Sha256.HMAC(k).update(input).digest() - return crypto_verify_32(h, 0, correct, 0) | sodium_memcmp(correct, h, 32) + return crypto_verify_32(h, 0, correct, 0) && sodium_memcmp(correct, h, 32) } function crypto_auth_hmacsha512 (out, input, k) { @@ -40,7 +40,7 @@ function crypto_auth_hmacsha512 (out, input, k) { function crypto_auth_hmacsha512_verify (h, input, k) { const correct = Sha512.HMAC(k).update(input).digest() - return crypto_verify_64(h, 0, correct, 0) | sodium_memcmp(correct, h, 64) + return crypto_verify_64(h, 0, correct, 0) && sodium_memcmp(correct, h, 64) } function crypto_auth_hmacsha512256 (out, input, k) { @@ -57,7 +57,7 @@ function crypto_auth_hmacsha512256 (out, input, k) { function crypto_auth_hmacsha512256_verify (h, input, k) { const correct = Sha512.HMAC(k).update(input).digest() - return crypto_verify_32(h, 0, correct, 0) | sodium_memcmp(correct.subarray(0, 32), h, 32) + return crypto_verify_32(h, 0, correct, 0) && sodium_memcmp(correct.subarray(0, 32), h, 32) } function crypto_auth (out, input, k) {