From a89adaebc36d35739f280cd3bcf99f1e8ac694ee Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Mon, 1 May 2023 06:07:19 +0200 Subject: [PATCH 1/3] src/Mayaqua/Secure.c: fix potential null pointer dereference found by coverity CID 343536 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking name suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 1339 if (name == NULL || data == NULL || size == 0) 1340 { 1341 sec->Error = SEC_ERROR_BAD_PARAMETER; 1342 return false; 1343 } --- src/Mayaqua/Secure.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Mayaqua/Secure.c b/src/Mayaqua/Secure.c index 18078cd5..7ff81386 100644 --- a/src/Mayaqua/Secure.c +++ b/src/Mayaqua/Secure.c @@ -1313,14 +1313,6 @@ bool WriteSecData(SECURE *sec, bool private_obj, char *name, void *data, UINT si UINT object_class = CKO_DATA; CK_BBOOL b_true = true, b_false = false, b_private_obj = private_obj; UINT object; - CK_ATTRIBUTE a[] = - { - {CKA_TOKEN, &b_true, sizeof(b_true)}, - {CKA_CLASS, &object_class, sizeof(object_class)}, - {CKA_PRIVATE, &b_private_obj, sizeof(b_private_obj)}, - {CKA_LABEL, name, StrLen(name)}, - {CKA_VALUE, data, size}, - }; // Validate arguments if (sec == NULL) { @@ -1347,6 +1339,15 @@ bool WriteSecData(SECURE *sec, bool private_obj, char *name, void *data, UINT si return false; } + CK_ATTRIBUTE a[] = + { + {CKA_TOKEN, &b_true, sizeof(b_true)}, + {CKA_CLASS, &object_class, sizeof(object_class)}, + {CKA_PRIVATE, &b_private_obj, sizeof(b_private_obj)}, + {CKA_LABEL, name, StrLen(name)}, + {CKA_VALUE, data, size}, + }; + // Delete any objects with the same name if (CheckSecObject(sec, name, SEC_DATA)) { From db7d6c83d5dc109eb7e1af35a1eb82b97f6b3f14 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Mon, 1 May 2023 06:09:38 +0200 Subject: [PATCH 2/3] src/Mayaqua/Secure.c: fix potential null pointer dereference found by coverity CID 343537 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking name suggests that it may be null but it has already been dereferenced on all paths leading to the check. 664 if (name == NULL) 665 { 666 sec->Error = SEC_ERROR_BAD_PARAMETER; 667 return false; 668 } --- src/Mayaqua/Secure.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/Mayaqua/Secure.c b/src/Mayaqua/Secure.c index 7ff81386..8008d051 100644 --- a/src/Mayaqua/Secure.c +++ b/src/Mayaqua/Secure.c @@ -640,22 +640,6 @@ bool WriteSecCert(SECURE *sec, bool private_obj, char *name, X *x) UINT ret; BUF *b; UINT object; - CK_ATTRIBUTE a[] = - { - {CKA_SUBJECT, subject, 0}, // 0 - {CKA_ISSUER, issuer, 0}, // 1 - {CKA_SERIAL_NUMBER, serial_number, 0}, // 2 - {CKA_VALUE, value, 0}, // 3 - {CKA_CLASS, &obj_class, sizeof(obj_class)}, - {CKA_TOKEN, &b_true, sizeof(b_true)}, - {CKA_PRIVATE, &b_private_obj, sizeof(b_private_obj)}, - {CKA_LABEL, name, StrLen(name)}, - {CKA_CERTIFICATE_TYPE, &cert_type, sizeof(cert_type)}, -#if 0 // Don't use these because some tokens fail - {CKA_START_DATE, &start_date, sizeof(start_date)}, - {CKA_END_DATE, &end_date, sizeof(end_date)}, -#endif - }; // Validate arguments if (sec == NULL) { @@ -677,6 +661,23 @@ bool WriteSecCert(SECURE *sec, bool private_obj, char *name, X *x) return false; } + CK_ATTRIBUTE a[] = + { + {CKA_SUBJECT, subject, 0}, // 0 + {CKA_ISSUER, issuer, 0}, // 1 + {CKA_SERIAL_NUMBER, serial_number, 0}, // 2 + {CKA_VALUE, value, 0}, // 3 + {CKA_CLASS, &obj_class, sizeof(obj_class)}, + {CKA_TOKEN, &b_true, sizeof(b_true)}, + {CKA_PRIVATE, &b_private_obj, sizeof(b_private_obj)}, + {CKA_LABEL, name, StrLen(name)}, + {CKA_CERTIFICATE_TYPE, &cert_type, sizeof(cert_type)}, +#if 0 // Don't use these because some tokens fail + {CKA_START_DATE, &start_date, sizeof(start_date)}, + {CKA_END_DATE, &end_date, sizeof(end_date)}, +#endif + }; + // Copy the certificate to the buffer b = XToBuf(x, false); if (b == NULL) From c59df8266697321dc02e76bb8a5545577e8fa542 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Mon, 1 May 2023 06:18:39 +0200 Subject: [PATCH 3/3] src/Mayaqua/Secure.c: fix potential null pointer dereference found by coverity CID 343528 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking name suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 438 if (name == NULL || k == NULL || k->private_key == false) 439 { 440 sec->Error = SEC_ERROR_BAD_PARAMETER; 441 return false; 442 } --- src/Mayaqua/Secure.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/Mayaqua/Secure.c b/src/Mayaqua/Secure.c index 8008d051..af2b9136 100644 --- a/src/Mayaqua/Secure.c +++ b/src/Mayaqua/Secure.c @@ -404,6 +404,28 @@ bool WriteSecKey(SECURE *sec, bool private_obj, char *name, K *k) UCHAR modules[MAX_SIZE], pub[MAX_SIZE], pri[MAX_SIZE], prime1[MAX_SIZE], prime2[MAX_SIZE]; UCHAR exp1[MAX_SIZE], exp2[MAX_SIZE], coeff[MAX_SIZE]; const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp; + + // Validate arguments + if (sec == NULL) + { + return false; + } + if (name == NULL || k == NULL || k->private_key == false) + { + sec->Error = SEC_ERROR_BAD_PARAMETER; + return false; + } + if (sec->SessionCreated == false) + { + sec->Error = SEC_ERROR_NO_SESSION; + return false; + } + if (sec->LoginFlag == false && private_obj) + { + sec->Error = SEC_ERROR_NOT_LOGIN; + return false; + } + CK_ATTRIBUTE a[] = { {CKA_MODULUS, modules, 0}, // 0 @@ -430,27 +452,6 @@ bool WriteSecKey(SECURE *sec, bool private_obj, char *name, K *k) {CKA_MODIFIABLE, &b_false, sizeof(b_false)}, }; - // Validate arguments - if (sec == NULL) - { - return false; - } - if (name == NULL || k == NULL || k->private_key == false) - { - sec->Error = SEC_ERROR_BAD_PARAMETER; - return false; - } - if (sec->SessionCreated == false) - { - sec->Error = SEC_ERROR_NO_SESSION; - return false; - } - if (sec->LoginFlag == false && private_obj) - { - sec->Error = SEC_ERROR_NOT_LOGIN; - return false; - } - // Numeric data generation rsa = EVP_PKEY_get0_RSA(k->pkey); if (rsa == NULL)