CVE-2018-16391: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Several buffer overflows when handling responses from a Muscle Card in muscle_list_files in libopensc/card-muscle.c in OpenSC before 0.19.0-rc1 could be used by attackers able to supply crafted smartcards to cause a denial of service (application crash) or possibly have unspecified other impact.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 6.8
Published: 2018-09-03T14:29Z
Repository: https://github.com/OpenSC/OpenSC
Language: C
Files changed: src/libopensc/card-cac.c, src/libopensc/card-epass2003.c, src/libopensc/card-muscle.c, src/libopensc/card-tcos.c, src/libopensc/pkcs15-esteid.c, src/libopensc/pkcs15-gemsafeV1.c, src/libopensc/pkcs15-sc-hsm.c, src/libopensc/sc.c, src/tools/cryptoflex-tool.c, src/tools/egk-tool.c, src/tools/util.c
Commit date: 2018-08-14 15:50:13 +0200
Commit message: fixed out of bounds writes

Thanks to Eric Sesterhenn from X41 D-SEC GmbH
for reporting the problems.

Diff: diff --git a/src/libopensc/card-cac.c b/src/libopensc/card-cac.c
index a30a0438..756788a0 100644
--- a/src/libopensc/card-cac.c
+++ b/src/libopensc/card-cac.c
@@ -792,11 +792,11 @@ static int cac_get_serial_nr_from_CUID(sc_card_t* card, sc_serial_number_t* seri
                 *serial = card->serialnr;
                 SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_SUCCESS);
         }
 	if (priv->cac_id_len) {
 		serial->len = MIN(priv->cac_id_len, SC_MAX_SERIALNR);
-		memcpy(serial->value, priv->cac_id, priv->cac_id_len);
+		memcpy(serial->value, priv->cac_id, serial->len);
 		SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_SUCCESS);
 	}
 	SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_FILE_NOT_FOUND);
 }
 
diff --git a/src/libopensc/card-epass2003.c b/src/libopensc/card-epass2003.c
index cbe417cf..65a58f67 100644
--- a/src/libopensc/card-epass2003.c
+++ b/src/libopensc/card-epass2003.c
@@ -949,11 +949,11 @@ decrypt_response(struct sc_card *card, unsigned char *in, size_t inlen, unsigned
 
 	/* unpadding */
 	while (0x80 != plaintext[cipher_len - 2] && (cipher_len - 2 > 0))
 		cipher_len--;
 
-	if (2 == cipher_len)
+	if (2 == cipher_len || *out_len < cipher_len - 2)
 		return -1;
 
 	memcpy(out, plaintext, cipher_len - 2);
 	*out_len = cipher_len - 2;
 	return 0;
@@ -975,10 +975,11 @@ epass2003_sm_unwrap_apdu(struct sc_card *card, struct sc_apdu *sm, struct sc_apd
 	LOG_FUNC_CALLED(card->ctx);
 
 	r = sc_check_sw(card, sm->sw1, sm->sw2);
 	if (r == SC_SUCCESS) {
 		if (exdata->sm) {
+			len = plain->resplen;
 			if (0 != decrypt_response(card, sm->resp, sm->resplen, plain->resp, &len))
 				return SC_ERROR_CARD_CMD_FAILED;
 		}
 		else {
 			memcpy(plain->resp, sm->resp, sm->resplen);
diff --git a/src/libopensc/card-muscle.c b/src/libopensc/card-muscle.c
index fd148e30..d247efab 100644
--- a/src/libopensc/card-muscle.c
+++ b/src/libopensc/card-muscle.c
@@ -516,20 +516,23 @@ static int muscle_list_files(sc_card_t *card, u8 *buf, size_t bufLen)
 	int count = 0;
 
 	mscfs_check_cache(priv->fs);
 
 	for(x = 0; x < fs->cache.size; x++) {
-		u8* oid= fs->cache.array[x].objectId.id;
+		u8* oid = fs->cache.array[x].objectId.id;
+		if (bufLen < 2)
+			break;
 		sc_debug(card->ctx, SC_LOG_DEBUG_NORMAL,
 			"FILE: %02X%02X%02X%02X\n",
 			oid[0],oid[1],oid[2],oid[3]);
 		if(0 == memcmp(fs->currentPath, oid, 2)) {
 			buf[0] = oid[2];
 			buf[1] = oid[3];
 			if(buf[0] == 0x00 && buf[1] == 0x00) continue; /* No directories/null names outside of root */
 			buf += 2;
-			count+=2;
+			count += 2;
+			bufLen -= 2;
 		}
 	}
 	return count;
 }
 
diff --git a/src/libopensc/card-tcos.c b/src/libopensc/card-tcos.c
index 40eac877..3c86b1f4 100644
--- a/src/libopensc/card-tcos.c
+++ b/src/libopensc/card-tcos.c
@@ -406,11 +406,11 @@ static int tcos_select_file(sc_card_t *card,
 	if (file == NULL) SC_FUNC_RETURN(ctx, SC_LOG_DEBUG_NORMAL, SC_ERROR_OUT_OF_MEMORY);
 	*file_out = file;
 	file->path = *in_path;
 
 	for(i=2; i+1<apdu.resplen && i+1+apdu.resp[i+1]<apdu.resplen; i+=2+apdu.resp[i+1]){
-		int j, len=apdu.resp[i+1];
+		size_t j, len=apdu.resp[i+1];
 		unsigned char type=apdu.resp[i], *d=apdu.resp+i+2;
 
 		switch (type) {
 		case 0x80:
 		case 0x81:
@@ -430,12 +430,12 @@ static int tcos_select_file(sc_card_t *card,
 			break;
 		case 0x83:
 			file->id = (d[0]<<8) | d[1];
 			break;
 		case 0x84:
-			memcpy(file->name, d, len);
-			file->namelen = len;
+			file->namelen = MIN(sizeof file->name, len);
+			memcpy(file->name, d, file->namelen);
 			break;
 		case 0x86:
 			sc_file_set_sec_attr(file, d, len); 
 			break;
 		default:
diff --git a/src/libopensc/pkcs15-esteid.c b/src/libopensc/pkcs15-esteid.c
index b3cf5178..2b8d66b9 100644
--- a/src/libopensc/pkcs15-esteid.c
+++ b/src/libopensc/pkcs15-esteid.c
@@ -77,11 +77,11 @@ sc_pkcs15emu_esteid_init (sc_pkcs15_card_t * p15card)
 	SC_TEST_RET(card->ctx, SC_LOG_DEBUG_NORMAL, r, "select esteid PD failed");
 
 	/* read the serial (document number) */
 	r = sc_read_record (card, SC_ESTEID_PD_DOCUMENT_NR, buff, sizeof(buff), SC_RECORD_BY_REC_NR);
 	SC_TEST_RET(card->ctx, SC_LOG_DEBUG_NORMAL, r, "read document number failed");
-	buff[r] = '\0';
+	buff[MIN((size_t) r, (sizeof buff)-1)] = '\0';
 	set_string (&p15card->tokeninfo->serial_number, (const char *) buff);
 
 	p15card->tokeninfo->flags = SC_PKCS15_TOKEN_PRN_GENERATION
 				  | SC_PKCS15_TOKEN_EID_COMPLIANT
 				  | SC_PKCS15_TOKEN_READONLY;
diff --git a/src/libopensc/pkcs15-gemsafeV1.c b/src/libopensc/pkcs15-gemsafeV1.c
index a8162b9d..3b220f7a 100644
--- a/src/libopensc/pkcs15-gemsafeV1.c
+++ b/src/libopensc/pkcs15-gemsafeV1.c
@@ -206,11 +206,11 @@ static int gemsafe_get_cert_len(sc_card_t *card)
 	 * For allocated keys, the fourth byte seems to indicate the
 	 * default key and the fifth byte indicates the key_ref of
 	 * the private key.
 	 */
 	ind = 2; /* skip length */
-	while (ibuf[ind] == 0x01) {
+	while (ibuf[ind] == 0x01 && i < gemsafe_cert_max) {
 		if (ibuf[ind+1] == 0xFE) {
 			gemsafe_prkeys[i].ref = ibuf[ind+4];
 			sc_log(card->ctx, "Key container %d is allocated and uses key_ref %d",
 					i+1, gemsafe_prkeys[i].ref);
 			ind += 9;
diff --git a/src/libopensc/pkcs15-sc-hsm.c b/src/libopensc/pkcs15-sc-hsm.c
index 305572cb..1391bea1 100644
--- a/src/libopensc/pkcs15-sc-hsm.c
+++ b/src/libopensc/pkcs15-sc-hsm.c
@@ -835,16 +835,18 @@ static int sc_pkcs15emu_sc_hsm_init (sc_pkcs15_card_t * p15card)
 	} else {
 		len = sizeof efbin;
 		r = read_file(p15card, (u8 *) "\x2F\x02", efbin, &len, 1);
 		LOG_TEST_RET(card->ctx, r, "Skipping optional EF.C_DevAut");
 
-		/* save EF_C_DevAut for further use */
-		ptr = realloc(priv->EF_C_DevAut, len);
-		if (ptr) {
-			memcpy(ptr, efbin, len);
-			priv->EF_C_DevAut = ptr;
-			priv->EF_C_DevAut_len = len;
+		if (len > 0) {
+			/* save EF_C_DevAut for further use */
+			ptr = realloc(priv->EF_C_DevAut, len);
+			if (ptr) {
+				memcpy(ptr, efbin, len);
+				priv->EF_C_DevAut = ptr;
+				priv->EF_C_DevAut_len = len;
+			}
 		}
 
 		ptr = efbin;
 	}
 
diff --git a/src/libopensc/sc.c b/src/libopensc/sc.c
index af71cafa..53fefb3e 100644
--- a/src/libopensc/sc.c
+++ b/src/libopensc/sc.c
@@ -626,11 +626,11 @@ int sc_file_set_sec_attr(sc_file_t *file, const u8 *sec_attr,
 	u8 *tmp;
 	if (!sc_file_valid(file)) {
 		return SC_ERROR_INVALID_ARGUMENTS;
 	}
 
-	if (sec_attr == NULL) {
+	if (sec_attr == NULL || sec_attr_len) {
 		if (file->sec_attr != NULL)
 			free(file->sec_attr);
 		file->sec_attr = NULL;
 		file->sec_attr_len = 0;
 		return 0;
diff --git a/src/tools/cryptoflex-tool.c b/src/tools/cryptoflex-tool.c
index 21b0baa1..662a0282 100644
--- a/src/tools/cryptoflex-tool.c
+++ b/src/tools/cryptoflex-tool.c
@@ -19,10 +19,11 @@
  */
 
 #include "config.h"
 
 #include "libopensc/sc-ossl-compat.h"
+#include "libopensc/internal.h"
 #include <openssl/bn.h>
 #include <openssl/rsa.h>
 #include <openssl/x509.h>
 #include <openssl/pem.h>
 #include <openssl/err.h>
@@ -329,11 +330,11 @@ static int read_public_key(RSA *rsa)
 	r = sc_select_file(card, &path, &file);
 	if (r) {
 		fprintf(stderr, "Unable to select public key file: %s\n", sc_strerror(r));
 		return 2;
 	}
-	bufsize = file->size;
+	bufsize = MIN(file->size, sizeof buf);
 	sc_file_free(file);
 	r = sc_read_binary(card, 0, buf, bufsize, 0);
 	if (r < 0) {
 		fprintf(stderr, "Unable to read public key file: %s\n", sc_strerror(r));
 		return 2;
@@ -380,11 +381,11 @@ static int read_private_key(RSA *rsa)
 		return 2;
 	}
 	e = sc_file_get_acl_entry(file, SC_AC_OP_READ);
 	if (e == NULL || e->method == SC_AC_NEVER)
 		return 10;
-	bufsize = file->size;
+	bufsize = MIN(file->size, sizeof buf);
 	sc_file_free(file);
 	r = sc_read_binary(card, 0, buf, bufsize, 0);
 	if (r < 0) {
 		fprintf(stderr, "Unable to read private key file: %s\n", sc_strerror(r));
 		return 2;
diff --git a/src/tools/egk-tool.c b/src/tools/egk-tool.c
index 0a8834b6..31360833 100644
--- a/src/tools/egk-tool.c
+++ b/src/tools/egk-tool.c
@@ -147,11 +147,11 @@ int read_file(struct sc_card *card, char *str_path, unsigned char **data, size_t
 	sc_format_path(str_path, &path);
 	if (SC_SUCCESS != sc_select_file(card, &path, &file)) {
 		goto err;
 	}
 
-	len = file ? file->size : 4096;
+	len = file && file->size > 0 ? file->size : 4096;
 	p = realloc(*data, len);
 	if (!p) {
 		goto err;
 	}
 	*data = p;
diff --git a/src/tools/util.c b/src/tools/util.c
index 6b8743e2..e49647a4 100644
--- a/src/tools/util.c
+++ b/src/tools/util.c
@@ -337,14 +337,15 @@ const char * util_acl_to_str(const sc_acl_entry_t *e)
 			break;
 		default:
 			strcpy(buf, "????");
 			break;
 		}
-		strcat(line, buf);
-		strcat(line, " ");
+		strncat(line, buf, sizeof line);
+		strncat(line, " ", sizeof line);
 		e = e->next;
 	}
+	line[(sizeof line)-1] = '\0'; /* make sure it's NUL terminated */
 	line[strlen(line)-1] = 0; /* get rid of trailing space */
 	return line;
 }
 
 NORETURN void

Vulnerable code: 		memcpy(serial->value, priv->cac_id, priv->cac_id_len);
	if (2 == cipher_len)
		u8* oid= fs->cache.array[x].objectId.id;
			count+=2;
		int j, len=apdu.resp[i+1];
			memcpy(file->name, d, len);
			file->namelen = len;
	buff[r] = '\0';
	while (ibuf[ind] == 0x01) {
		/* save EF_C_DevAut for further use */
		ptr = realloc(priv->EF_C_DevAut, len);
		if (ptr) {
			memcpy(ptr, efbin, len);
			priv->EF_C_DevAut = ptr;
			priv->EF_C_DevAut_len = len;
	if (sec_attr == NULL) {
	bufsize = file->size;
	bufsize = file->size;
	len = file ? file->size : 4096;
		strcat(line, buf);
		strcat(line, " ");
Fixed code: 		memcpy(serial->value, priv->cac_id, serial->len);
	if (2 == cipher_len || *out_len < cipher_len - 2)
			len = plain->resplen;
		u8* oid = fs->cache.array[x].objectId.id;
		if (bufLen < 2)
			break;
			count += 2;
			bufLen -= 2;
		size_t j, len=apdu.resp[i+1];
			file->namelen = MIN(sizeof file->name, len);
			memcpy(file->name, d, file->namelen);
	buff[MIN((size_t) r, (sizeof buff)-1)] = '\0';
	while (ibuf[ind] == 0x01 && i < gemsafe_cert_max) {
		if (len > 0) {
			/* save EF_C_DevAut for further use */
			ptr = realloc(priv->EF_C_DevAut, len);
			if (ptr) {
				memcpy(ptr, efbin, len);
				priv->EF_C_DevAut = ptr;
				priv->EF_C_DevAut_len = len;
			}
	if (sec_attr == NULL || sec_attr_len) {
#include "libopensc/internal.h"
	bufsize = MIN(file->size, sizeof buf);
	bufsize = MIN(file->size, sizeof buf);
	len = file && file->size > 0 ? file->size : 4096;
		strncat(line, buf, sizeof line);
		strncat(line, " ", sizeof line);
	line[(sizeof line)-1] = '\0'; /* make sure it's NUL terminated */