From e36c2cf16164cdbbd52615d5b07a4e73d1f60bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= Date: Wed, 22 May 2024 10:27:00 +0200 Subject: [PATCH] Fix SAST findings Resolves: RHEL-36768 --- opensc-0.25.1-sast.patch | 186 +++++++++++++++++++++++++++++++++++++++ opensc.spec | 11 ++- 2 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 opensc-0.25.1-sast.patch diff --git a/opensc-0.25.1-sast.patch b/opensc-0.25.1-sast.patch new file mode 100644 index 0000000..4e8b220 --- /dev/null +++ b/opensc-0.25.1-sast.patch @@ -0,0 +1,186 @@ +From 86eabd42ea7c93d23258c70ea79ea3ed4f3110f8 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= +Date: Wed, 22 May 2024 09:55:04 +0200 +Subject: [PATCH 1/4] card-piv: Fix integer overflow + +--- + src/libopensc/card-piv.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/src/libopensc/card-piv.c b/src/libopensc/card-piv.c +index c3a305799..ca071c58e 100644 +--- a/src/libopensc/card-piv.c ++++ b/src/libopensc/card-piv.c +@@ -3408,11 +3408,11 @@ piv_put_data(sc_card_t *card, int tag, const u8 *buf, size_t buf_len) + SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE); + + tag_len = piv_objects[tag].tag_len; +- sbuflen = sc_asn1_put_tag(0x5c, piv_objects[tag].tag_value, tag_len, NULL, 0, NULL); +- if (sbuflen <= 0) { ++ r = sc_asn1_put_tag(0x5c, piv_objects[tag].tag_value, tag_len, NULL, 0, NULL); ++ if (r <= 0) { + LOG_FUNC_RETURN(card->ctx, SC_ERROR_INTERNAL); + } +- sbuflen += buf_len; ++ sbuflen = r + buf_len; + if (!(sbuf = malloc(sbuflen))) { + LOG_FUNC_RETURN(card->ctx, SC_ERROR_OUT_OF_MEMORY); + } +-- +2.44.0 + + +From 6923b97d59f8a5a636223a36465912e0c640a534 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= +Date: Wed, 22 May 2024 10:11:46 +0200 +Subject: [PATCH 2/4] iso7816.c: Check length of file_ref to prevent buffer + overrun + +--- + src/libopensc/iso7816.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/libopensc/iso7816.c b/src/libopensc/iso7816.c +index 73893e941..98c059bdc 100644 +--- a/src/libopensc/iso7816.c ++++ b/src/libopensc/iso7816.c +@@ -1003,7 +1003,7 @@ iso7816_set_security_env(struct sc_card *card, + *p++ = env->algorithm_ref & 0xFF; + } + if (env->flags & SC_SEC_ENV_FILE_REF_PRESENT) { +- if (env->file_ref.len > 0xFF) ++ if (env->file_ref.len > SC_MAX_PATH_SIZE) + return SC_ERROR_INVALID_ARGUMENTS; + if (sizeof(sbuf) - (p - sbuf) < env->file_ref.len + 2) + return SC_ERROR_OFFSET_TOO_LARGE; +-- +2.44.0 + + +From 6ceb50e271f4b67ec934aae5f8383475e1bd58ea Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= +Date: Thu, 23 May 2024 10:04:08 +0200 +Subject: [PATCH 3/4] iso7816.c: Check length of key_ref_len to prevent buffer + overrun + +--- + src/libopensc/iso7816.c | 2 +- + src/libopensc/opensc.h | 2 +- + src/libopensc/types.h | 1 + + 3 files changed, 3 insertions(+), 2 deletions(-) + +diff --git a/src/libopensc/iso7816.c b/src/libopensc/iso7816.c +index 98c059bdc..d7e044993 100644 +--- a/src/libopensc/iso7816.c ++++ b/src/libopensc/iso7816.c +@@ -1021,7 +1021,7 @@ iso7816_set_security_env(struct sc_card *card, + *p++ = 0x83; + else + *p++ = 0x84; +- if (env->key_ref_len > 0xFF) ++ if (env->key_ref_len > SC_MAX_KEYREF_SIZE) + return SC_ERROR_INVALID_ARGUMENTS; + *p++ = env->key_ref_len & 0xFF; + memcpy(p, env->key_ref, env->key_ref_len); +diff --git a/src/libopensc/opensc.h b/src/libopensc/opensc.h +index cbcb3bebb..5faf590ae 100644 +--- a/src/libopensc/opensc.h ++++ b/src/libopensc/opensc.h +@@ -251,7 +251,7 @@ typedef struct sc_security_env { + + unsigned long algorithm_ref; + struct sc_path file_ref; +- unsigned char key_ref[8]; ++ unsigned char key_ref[SC_MAX_KEYREF_SIZE]; + size_t key_ref_len; + struct sc_path target_file_ref; /* target key file in unwrap operation */ + +diff --git a/src/libopensc/types.h b/src/libopensc/types.h +index 7eea3fa82..304725a67 100644 +--- a/src/libopensc/types.h ++++ b/src/libopensc/types.h +@@ -50,6 +50,7 @@ typedef unsigned char u8; + #define SC_MAX_CRTS_IN_SE 12 + #define SC_MAX_SE_NUM 8 + #define SC_MAX_PKCS15_EMULATORS 48 ++#define SC_MAX_KEYREF_SIZE 8 + + /* When changing this value, pay attention to the initialization of the ASN1 + * static variables that use this macro, like, for example, +-- +2.44.0 + + +From 42d4950b9804a6c057586e0e6cb614205818c411 Mon Sep 17 00:00:00 2001 +From: Jakub Jelen +Date: Thu, 9 May 2024 18:53:15 +0200 +Subject: [PATCH 4/4] pkcs15init: Avoid memory leaks + +--- + src/pkcs15init/pkcs15-lib.c | 23 +++++++++++++++-------- + 1 file changed, 15 insertions(+), 8 deletions(-) + +diff --git a/src/pkcs15init/pkcs15-lib.c b/src/pkcs15init/pkcs15-lib.c +index 0dc62af4a..dbb5748b2 100644 +--- a/src/pkcs15init/pkcs15-lib.c ++++ b/src/pkcs15init/pkcs15-lib.c +@@ -1888,8 +1888,10 @@ sc_pkcs15init_store_public_key(struct sc_pkcs15_card *p15card, struct sc_profile + key.u.rsa.modulus.data = NULL; + key.u.rsa.exponent.data = NULL; + // copy RSA params +- if (!(key.u.rsa.modulus.data = malloc(keyargs->key.u.rsa.modulus.len))) +- LOG_TEST_RET(ctx, SC_ERROR_OUT_OF_MEMORY, "Failed to copy RSA public key parameters"); ++ if (!(key.u.rsa.modulus.data = malloc(keyargs->key.u.rsa.modulus.len))) { ++ r = SC_ERROR_OUT_OF_MEMORY; ++ LOG_TEST_GOTO_ERR(ctx, r, "Failed to copy RSA public key parameters"); ++ } + memcpy(key.u.rsa.modulus.data, keyargs->key.u.rsa.modulus.data, keyargs->key.u.rsa.modulus.len); + if (!(key.u.rsa.exponent.data = malloc(keyargs->key.u.rsa.exponent.len))) { + r = SC_ERROR_OUT_OF_MEMORY; +@@ -1902,8 +1904,10 @@ sc_pkcs15init_store_public_key(struct sc_pkcs15_card *p15card, struct sc_profile + case SC_ALGORITHM_GOSTR3410: + key.u.gostr3410.xy.data = NULL; + // copy GOSTR params +- if (!(key.u.gostr3410.xy.data = malloc(keyargs->key.u.gostr3410.xy.len))) +- LOG_TEST_RET(ctx, SC_ERROR_OUT_OF_MEMORY, "Failed to copy GOSTR public key parameters"); ++ if (!(key.u.gostr3410.xy.data = malloc(keyargs->key.u.gostr3410.xy.len))) { ++ r = SC_ERROR_OUT_OF_MEMORY; ++ LOG_TEST_GOTO_ERR(ctx, r, "Failed to copy GOSTR public key parameters"); ++ } + memcpy(key.u.gostr3410.xy.data, keyargs->key.u.gostr3410.xy.data, keyargs->key.u.gostr3410.xy.len); + keybits = SC_PKCS15_GOSTR3410_KEYSIZE; + type = SC_PKCS15_TYPE_PUBKEY_GOSTR3410; +@@ -1912,14 +1916,15 @@ sc_pkcs15init_store_public_key(struct sc_pkcs15_card *p15card, struct sc_profile + type = SC_PKCS15_TYPE_PUBKEY_EC; + + r = sc_copy_ec_params(&key.u.ec.params, &keyargs->key.u.ec.params); +- LOG_TEST_RET(ctx, r, "Failed to copy EC public key parameters"); ++ LOG_TEST_GOTO_ERR(ctx, r, "Failed to copy EC public key parameters"); + r = sc_pkcs15_fix_ec_parameters(ctx, &key.u.ec.params); + LOG_TEST_GOTO_ERR(ctx, r, "Failed to fix EC public key parameters"); + + keybits = key.u.ec.params.field_length; + break; + default: +- LOG_TEST_RET(ctx, SC_ERROR_NOT_SUPPORTED, "Unsupported key algorithm."); ++ r = SC_ERROR_NOT_SUPPORTED; ++ LOG_TEST_GOTO_ERR(ctx, r, "Unsupported key algorithm."); + } + + if ((usage = keyargs->usage) == 0) { +@@ -1933,8 +1938,10 @@ sc_pkcs15init_store_public_key(struct sc_pkcs15_card *p15card, struct sc_profile + + /* Set up the pkcs15 object. */ + object = sc_pkcs15init_new_object(type, label, &keyargs->auth_id, NULL); +- if (object == NULL) +- LOG_TEST_RET(ctx, SC_ERROR_OUT_OF_MEMORY, "Cannot allocate new public key object"); ++ if (object == NULL) { ++ r = SC_ERROR_OUT_OF_MEMORY; ++ LOG_TEST_GOTO_ERR(ctx, r, "Cannot allocate new public key object"); ++ } + + key_info = (struct sc_pkcs15_pubkey_info *) object->data; + key_info->usage = usage; +-- +2.44.0 + diff --git a/opensc.spec b/opensc.spec index 9db6a95..d8bbed2 100644 --- a/opensc.spec +++ b/opensc.spec @@ -1,6 +1,6 @@ Name: opensc Version: 0.25.1 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Smart card library and applications License: LGPL-2.1-or-later AND BSD-3-Clause @@ -10,6 +10,11 @@ Source1: opensc.module Patch1: opensc-0.19.0-pinpad.patch # File caching by default (#2000626) Patch8: %{name}-0.22.0-file-cache.patch +# https://github.com/OpenSC/OpenSC/pull/3153/commits/27033005fc41c86f3b7d436ab74774bad2bc4727 +# https://github.com/OpenSC/OpenSC/pull/3153/commits/2761afdecabddd8ab4ab5e079d046611168ce7b0 +# https://github.com/OpenSC/OpenSC/pull/3153/commits/c6d4881228e359cf4da24003608890367366be26 +# https://github.com/OpenSC/OpenSC/pull/3143/commits/d3ae13af0b198f2cfbc254cc23f66d3790d76c0c +Patch9: %{name}-0.25.1-sast.patch BuildRequires: make BuildRequires: pcsc-lite-devel @@ -53,6 +58,7 @@ every software/card that does so, too. %setup -q %patch 1 -p1 -b .pinpad %patch 8 -p1 -b .file-cache +%patch 9 -p1 -b .sast # The test-pkcs11-tool-allowed-mechanisms already works in Fedora sed -i -e '/XFAIL_TESTS/,$ { @@ -203,6 +209,9 @@ rm %{buildroot}%{_mandir}/man1/opensc-notify.1* %changelog +* Thu Jun 12 2024 Veronika Hanulikova - 0.25.1-3 +- Fix SAST findings (RHEL-36768) + * Tue Apr 16 2024 Veronika Hanulikova - 0.25.1-2 - Fix license identifier in spec file