Skip to content

Commit

Permalink
kelektivGH-720 - change to strcmp.
Browse files Browse the repository at this point in the history
The prior code didn't provide constant time comparison and it wasn't
necessary anyway. Removed in favor of strcmp. Kept the wrapper function
to maintain api consistency.

Signed-off-by: Nick Campbell <nicholas.j.campbell@gmail.com>
  • Loading branch information
ncb000gt authored and recrsn committed Dec 30, 2019
1 parent 39793c9 commit 2f9b1dc
Showing 1 changed file with 1 addition and 20 deletions.
21 changes: 1 addition & 20 deletions src/bcrypt_node.cc
Expand Up @@ -248,26 +248,7 @@ NAN_METHOD(EncryptSync) {
/* COMPARATOR */

NAN_INLINE bool CompareStrings(const char* s1, const char* s2) {

bool eq = true;
int s1_len = strlen(s1);
int s2_len = strlen(s2);

if (s1_len != s2_len) {
eq = false;
}

const int max_len = (s2_len < s1_len) ? s1_len : s2_len;

// to prevent timing attacks, should check entire string
// don't exit after found to be false
for (int i = 0; i < max_len; ++i) {
if (s1_len >= i && s2_len >= i && s1[i] != s2[i]) {
eq = false;
}
}

return eq;
return strcmp(s1, s2) == 0;
}

class CompareAsyncWorker : public Nan::AsyncWorker {
Expand Down

0 comments on commit 2f9b1dc

Please sign in to comment.