Skip to content

Commit

Permalink
GH-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 Aug 20, 2019
1 parent dd0798c commit c8c8562
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 c8c8562

Please sign in to comment.