Merge pull request #496 from nichtsfrei/gvm-libs-20.08

When hash or password are NULL return INVALID rather then ERR

CHANGELOG.md

@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Changed
 Use a char pointer instead of an zero-lenght array as kb_redis struct member. [443](https://github.com/greenbone/gvm-libs/pull/443)
+- pba verify returns INVALID instead of ERR when hash or password are null [496](https://github.com/greenbone/gvm-libs/pull/496)
 
 ### Fixed
 - Fixing [#434](https://github.com/greenbone/gvm-libs/pull/434) by removing the extra parentheses in `base/networking.c` [#437](https://github.com/greenbone/gvm-libs/pull/437)

util/passwordbasedauthentication.c

@@ -27,6 +27,11 @@
 // this shouldn't affect other implementations
 #define __USE_GNU
 #include <crypt.h>
+// INVALID_HASH is used on verify when the given hash is a NULL pointer.
+// This is done to not directly jump to exit with a INVALID_HASH result
+// but rather keep calculating to make it a little bit harder to guess
+// if a user exists or not based on timing.
+#define INVALID_HASH "1234567890$"
 #ifndef CRYPT_GENSALT_OUTPUT_SIZE
 #define CRYPT_GENSALT_OUTPUT_SIZE 192
 #endif
@@ -163,7 +168,7 @@ pba_is_phc_compliant (const char *setting)
 {
   if (setting == NULL)
     {
-      return 0;
+      return 1;
     }
   return strlen (setting) > 1 && setting[0] == '$';
 }
@@ -223,7 +228,7 @@ pba_verify_hash (const struct PBASettings *setting, const char *hash,
   struct crypt_data *data = NULL;
   int i = 0;
   enum pba_rc result = ERR;
-  if (!setting || !hash || !password)
+  if (!setting )
     goto exit;
   if (!is_prefix_supported (setting->prefix))
     goto exit;
@@ -232,15 +237,18 @@ pba_verify_hash (const struct PBASettings *setting, const char *hash,
       data = calloc (1, sizeof (struct crypt_data));
       // manipulate hash to reapply pepper
       tmp = malloc ( CRYPT_OUTPUT_SIZE);
-      strncpy(tmp, hash, CRYPT_OUTPUT_SIZE);
+      strncpy(tmp, hash ? hash : INVALID_HASH, CRYPT_OUTPUT_SIZE);
       cmp = strrchr (tmp, '$');
       for (i = MAX_PEPPER_SIZE - 1; i > -1; i--)
         {
           cmp--;
           if (setting->pepper[i] != 0)
             cmp[0] = setting->pepper[i];
         }
-      cmp = crypt_r (password, tmp, data);
+      // some crypt_r implementations cannot handle if password is a
+      // NULL pointer and run into SEGMENTATION faults. 
+      // Therefore we set it to "" 
+      cmp = crypt_r (password ? password : "", tmp, data);
       if (strcmp (tmp, cmp) == 0)
         result = VALID;
       else

util/passwordbasedauthentication_tests.c

@@ -33,7 +33,6 @@ AfterEach (PBA)
 
 Ensure (PBA, returns_false_on_not_phc_compliant_setting)
 {
-    assert_false(pba_is_phc_compliant(NULL));
     assert_false(pba_is_phc_compliant("$"));
     assert_false(pba_is_phc_compliant("password"));
 }
@@ -76,6 +75,17 @@ Ensure (PBA, verify_hash)
     free(hash);
 }
 
+Ensure (PBA, verify_hash_returns_invalid_on_np_hash_np_password)
+{
+    struct PBASettings setting = { "4242" , 20000, "$6$"};
+    char *hash;
+    hash = pba_hash(&setting, "*password");
+    assert_not_equal(hash, NULL);
+    assert_equal(pba_verify_hash(&setting, NULL, "*password"), INVALID);
+    assert_equal(pba_verify_hash(&setting, hash, NULL), INVALID);
+}
+
+
 Ensure (PBA, defaults)
 {
     int i;
@@ -131,6 +141,8 @@ main (int argc, char **argv)
                          unique_hash_without_adding_used_pepper);
   add_test_with_context (suite, PBA,
                          verify_hash);
+  add_test_with_context (suite, PBA,
+                         verify_hash_returns_invalid_on_np_hash_np_password);
   add_test_with_context (suite, PBA,
                          handle_md5_hash);
   add_test_with_context (suite, PBA,