diff --git a/src/bcrypt_node.cc b/src/bcrypt_node.cc index 2f072a4..c86edcb 100644 --- a/src/bcrypt_node.cc +++ b/src/bcrypt_node.cc @@ -22,7 +22,10 @@ namespace { // discard $ salt++; - if (*salt > BCRYPT_VERSION) { + // A short, truncated salt such as "$" or "$2" must be rejected before + // reading further offsets, otherwise the fixed lookups below would read + // past the terminating NUL byte. + if (*salt == '\0' || *salt > BCRYPT_VERSION) { return false; } @@ -37,10 +40,16 @@ namespace { } } + // the version must be followed by the '$' separator; bail out here so a + // salt like "$2b" cannot advance the pointer past its NUL terminator + if (salt[1] != '$') { + return false; + } + // discard version + $ salt += 2; - if (salt[2] != '$') { + if (salt[0] == '\0' || salt[1] == '\0' || salt[2] != '$') { return false; } diff --git a/test/sync.test.js b/test/sync.test.js index 2e6809a..0bea82d 100644 --- a/test/sync.test.js +++ b/test/sync.test.js @@ -70,6 +70,16 @@ test('hash_salt_validity', () => { expect(() => bcrypt.hashSync('password', 'some$value')).toThrow('Invalid salt. Salt must be in the form of: $Vers$log2(NumRounds)$saltvalue') }) +test('hash_short_salt_prefix', () => { + // Truncated "$"-prefixed salts must be rejected without reading past the + // end of the string (see ValidateSalt bounds handling). + const truncated = ['$', '$2', '$2b', '$2b$', '$2b$1', '$2b$10', '$2b$10$']; + for (const salt of truncated) { + expect(() => bcrypt.hashSync('password', salt)).toThrow('Invalid salt. Salt must be in the form of: $Vers$log2(NumRounds)$saltvalue') + expect(bcrypt.compareSync('password', salt)).toBe(false) + } +}) + test('verify_salt', () => { const salt = bcrypt.genSaltSync(10); const split_salt = salt.split('$');