From 83f45cda2af16b65264103fbe0394fd422f0120d Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Sun, 27 May 2018 00:38:37 +0200 Subject: [PATCH] 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 | 14 +++++++------- src/libopensc/errors.c | 2 ++ src/libopensc/errors.h | 2 ++ src/libopensc/simpletlv.c | 33 ++++++++++++++++++++++----------- src/tools/util.c | 5 +++-- 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/libopensc/card-setcos.c b/src/libopensc/card-setcos.c index 98464c2e..a8ca012f 100644 --- a/src/libopensc/card-setcos.c +++ b/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; - if ((size_t) iACLen > len) + size_t iACLen = buf[iOffset] & 0x0F; + 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 */ - int iKeyLen = 0; /* Encryption key is optional */ + size_t iParmLen = 1; /* command-byte is always present */ + 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]; diff --git a/src/libopensc/errors.c b/src/libopensc/errors.c index b6c6eb48..354e0a6e 100644 --- a/src/libopensc/errors.c +++ b/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; diff --git a/src/libopensc/errors.h b/src/libopensc/errors.h index 183b0758..e0330a93 100644 --- a/src/libopensc/errors.h +++ b/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 diff --git a/src/libopensc/simpletlv.c b/src/libopensc/simpletlv.c index ab0401b5..67a08da2 100644 --- a/src/libopensc/simpletlv.c +++ b/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 = p+buflen; - return SC_ERROR_INVALID_ARGUMENTS; - } + *buf = NULL; + + 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) { - *taglen = 0; - return SC_ERROR_INVALID_ARGUMENTS; + if (left < 2) { + return SC_ERROR_INVALID_TLV_OBJECT; } /* 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; } diff --git a/src/tools/util.c b/src/tools/util.c index e49647a4..a3cbff7a 100644 --- a/src/tools/util.c +++ b/src/tools/util.c @@ -31,6 +31,7 @@ #include #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); - strncat(line, " ", sizeof line); + strlcat(line, buf, sizeof line); + strlcat(line, " ", sizeof line); e = e->next; } line[(sizeof line)-1] = '\0'; /* make sure it's NUL terminated */