commit ead1766b5f910dd2a8d6368982b6539ed618634d
author Gilles Peskine <Gilles.Peskine@arm.com> Sun Aug 20 22:02:51 2023 +0200
committer Gilles Peskine <Gilles.Peskine@arm.com> Sun Aug 20 22:05:16 2023 +0200
tree 11d34bfbbb07bc655698cfa7c18890ece6c7771d
parent 73936868b84e19692b05c93f3662a24d1f7a04a1

Fix PBKDF2 with empty salt segment on platforms where malloc(0)=NULL

"Fix PBKDF2 with empty salt on platforms where malloc(0)=NULL" took care of
making an empty salt work. But it didn't fix the case of an empty salt
segment followed by a non-empty salt segment, which still invoked memcpy
with a potentially null pointer as the source. This commit fixes that case,
and also simplifies the logic in the function a little.

Test data obtained with:
```
pip3 install cryptodome
python3 -c 'import sys; from Crypto.Hash import SHA256; from Crypto.Protocol.KDF import PBKDF2; cost = int(sys.argv[1], 0); salt = bytes.fromhex(sys.argv[2]); password = bytes.fromhex(sys.argv[3]); n = int(sys.argv[4], 0); print(PBKDF2(password=password, salt=salt, dkLen=n, count=cost, hmac_hash_module=SHA256).hex())' 1 "" "706173737764" 64
```

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

library/psa_crypto.c

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index c4a48f8..35d71e7 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -6684,22 +6684,17 @@
                                         const uint8_t *data,
                                         size_t data_length)
 {
-    if (pbkdf2->state != PSA_PBKDF2_STATE_INPUT_COST_SET &&
-        pbkdf2->state != PSA_PBKDF2_STATE_SALT_SET) {
+    if (pbkdf2->state == PSA_PBKDF2_STATE_INPUT_COST_SET) {
+        pbkdf2->state = PSA_PBKDF2_STATE_SALT_SET;
+    } else if (pbkdf2->state == PSA_PBKDF2_STATE_SALT_SET) {
+        /* Appending to existing salt. No state change. */
+    } else {
         return PSA_ERROR_BAD_STATE;
     }
 
     if (data_length == 0) {
-        /* Nothing to do */
-    } else if (pbkdf2->state == PSA_PBKDF2_STATE_INPUT_COST_SET) {
-        pbkdf2->salt = mbedtls_calloc(1, data_length);
-        if (pbkdf2->salt == NULL) {
-            return PSA_ERROR_INSUFFICIENT_MEMORY;
-        }
-
-        memcpy(pbkdf2->salt, data, data_length);
-        pbkdf2->salt_length = data_length;
-    } else if (pbkdf2->state == PSA_PBKDF2_STATE_SALT_SET) {
+        /* Appending an empty string, nothing to do. */
+    } else {
         uint8_t *next_salt;
 
         next_salt = mbedtls_calloc(1, data_length + pbkdf2->salt_length);
@@ -6707,15 +6702,14 @@
             return PSA_ERROR_INSUFFICIENT_MEMORY;
         }
 
-        memcpy(next_salt, pbkdf2->salt, pbkdf2->salt_length);
+        if (pbkdf2->salt_length != 0) {
+            memcpy(next_salt, pbkdf2->salt, pbkdf2->salt_length);
+        }
         memcpy(next_salt + pbkdf2->salt_length, data, data_length);
         pbkdf2->salt_length += data_length;
         mbedtls_free(pbkdf2->salt);
         pbkdf2->salt = next_salt;
     }
-
-    pbkdf2->state = PSA_PBKDF2_STATE_SALT_SET;
-
     return PSA_SUCCESS;
 }