Added bounds checking to sc_simpletlv_read_tag()

- Logic is identical to sc_asn1_read_tag()
- Fixes out of bounds access e.g. in cac_parse_CCC

src/libopensc/card-setcos.c

@@ -788,8 +788,8 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len)
 
 	/* Check all sub-AC definitions within the total AC */
 	while (len > 1) {				/* minimum length = 2 */
-		int	iACLen   = buf[iOffset] & 0x0F;
+		size_t iACLen   = buf[iOffset] & 0x0F;
-		if ((size_t) iACLen > len)
+		if (iACLen > len)
 			break;
 
 		iPinCount = -1;			/* default no pin required */
@@ -797,8 +797,8 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len)
 
 		if (buf[iOffset] & 0X80) { /* AC in adaptive coding */
 			/* Evaluates only the command-byte, not the optional P1/P2/Option bytes */
-			int	iParmLen = 1;			/* command-byte is always present */
+			size_t	iParmLen = 1;			/* command-byte is always present */
-			int	iKeyLen  = 0;			/* Encryption key is optional */
+			size_t	iKeyLen  = 0;			/* Encryption key is optional */
 
 			if (buf[iOffset]   & 0x20) iKeyLen++;
 			if (buf[iOffset+1] & 0x40) iParmLen++;
@@ -809,7 +809,7 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len)
 			/* Get KeyNumber if available */
 			if(iKeyLen) {
 				int iSC;
-				if (len < 1+iACLen)
+				if (len < 1+(size_t)iACLen)
 					break;
 				iSC = buf[iOffset+iACLen];
 
@@ -830,7 +830,7 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len)
 
 			/* Get PinNumber if available */
 			if (iACLen > (1+iParmLen+iKeyLen)) {  /* check via total length if pin is present */
-				if (len < 1+1+1+iParmLen)
+				if (len < 1+1+1+(size_t)iParmLen)
 					break;
 				iKeyRef = buf[iOffset+1+1+iParmLen];  /* PTL + AM-header + parameter-bytes */
 				iMethod = SC_AC_CHV;
@@ -873,7 +873,7 @@ static void parse_sec_attr_44(sc_file_t *file, const u8 *buf, size_t len)
 
 			if (buf[iOffset] & 0x20) {
 				int iSC;
-				if (len < 1 + iACLen)
+				if (len < 1 + (size_t)iACLen)
 					break;
 				iSC = buf[iOffset + iACLen];
 

src/libopensc/errors.c

@@ -104,6 +104,8 @@ const char *sc_strerror(int error)
 		"Unable to load external module",
 		"EF offset too large",
 		"Not implemented"
+		"Invalid Simple TLV object",
+		"Premature end of Simple TLV stream",
 	};
 	const int int_base = -SC_ERROR_INTERNAL;
 

src/libopensc/errors.h

@@ -95,6 +95,8 @@ extern "C" {
 #define SC_ERROR_CANNOT_LOAD_MODULE		-1414
 #define SC_ERROR_OFFSET_TOO_LARGE		-1415
 #define SC_ERROR_NOT_IMPLEMENTED		-1416
+#define SC_ERROR_INVALID_TLV_OBJECT		-1417
+#define SC_ERROR_TLV_END_OF_CONTENTS	-1418
 
 /* Relating to PKCS #15 init stuff */
 #define SC_ERROR_PKCS15INIT			-1500

src/libopensc/simpletlv.c

@@ -74,27 +74,38 @@ sc_simpletlv_put_tag(u8 tag, size_t datalen, u8 *out, size_t outlen, u8 **ptr)
 int
 sc_simpletlv_read_tag(u8 **buf, size_t buflen, u8 *tag_out, size_t *taglen)
 {
-	size_t len;
+	u8 tag;
+	size_t left = buflen, len;
 	u8 *p = *buf;
 
-	if (buflen < 2) {
+	*buf = NULL;
-		*buf = p+buflen;
+
-		return SC_ERROR_INVALID_ARGUMENTS;
+	if (left < 2) {
-	}
+		return SC_ERROR_INVALID_TLV_OBJECT;
+	}
+	tag = *p;
+	p++;
+	len = *p;
+	p++;
+	left -= 2;
 
-	*tag_out = *p++;
-	len = *p++;
 	if (len == 0xff) {
 		/* don't crash on bad data */
-		if (buflen < 4) {
+		if (left < 2) {
-			*taglen = 0;
+			return SC_ERROR_INVALID_TLV_OBJECT;
-			return SC_ERROR_INVALID_ARGUMENTS;
 		}
 		/* skip two bytes (the size) */
 		len = lebytes2ushort(p);
-		p+=2;
+		p += 2;
+		left -= 2;
 	}
+
+	*tag_out = tag;
 	*taglen = len;
 	*buf = p;
+
+	if (len > left)
+		return SC_ERROR_TLV_END_OF_CONTENTS;
+
 	return SC_SUCCESS;
 }

src/tools/util.c

@@ -31,6 +31,7 @@
 #include <ctype.h>
 #include "util.h"
 #include "ui/notify.h"
+#include "common/compat_strlcat.h"
 
 int
 is_string_valid_atr(const char *atr_str)
@@ -339,8 +340,8 @@ const char * util_acl_to_str(const sc_acl_entry_t *e)
 			strcpy(buf, "????");
 			break;
 		}
-		strncat(line, buf, sizeof line);
+		strlcat(line, buf, sizeof line);
-		strncat(line, " ", sizeof line);
+		strlcat(line, " ", sizeof line);
 		e = e->next;
 	}
 	line[(sizeof line)-1] = '\0'; /* make sure it's NUL terminated */