From 4389d271cce517d4e0a84793f6822dee16980e22 Mon Sep 17 00:00:00 2001 From: Levi Rak Date: Fri, 16 Jun 2017 11:02:06 -0600 Subject: [PATCH 1/4] Fixed potential buffer overflows when configured with --enable-opensslextra --- src/ssl.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 9c5207f45..200106f4d 100755 --- a/src/ssl.c +++ b/src/ssl.c @@ -4152,22 +4152,28 @@ int PemToDer(const unsigned char* buff, long longSz, int type, { /* remove encrypted header if there */ char encHeader[] = "Proc-Type"; - char* line = XSTRNSTR(headerEnd, encHeader, PEM_LINE_LEN); + unsigned int headerEndSz = min(PEM_LINE_LEN, bufferEnd - headerEnd); + char* line = XSTRNSTR(headerEnd, encHeader, headerEndSz); + unsigned int lineSz = min(PEM_LINE_LEN, bufferEnd - line); if (line) { char* newline; char* finish; - char* start = XSTRNSTR(line, "DES", PEM_LINE_LEN); + char* start = XSTRNSTR(line, "DES", lineSz); + unsigned int finishSz; + unsigned int startSz; if (!start) - start = XSTRNSTR(line, "AES", PEM_LINE_LEN); + start = XSTRNSTR(line, "AES", lineSz); if (!start) return SSL_BAD_FILE; if (!info) return SSL_BAD_FILE; - finish = XSTRNSTR(start, ",", PEM_LINE_LEN); + startSz = min(PEM_LINE_LEN, bufferEnd - start); + finish = XSTRNSTR(start, ",", startSz); if (start && finish && (start < finish)) { - newline = XSTRNSTR(finish, "\r", PEM_LINE_LEN); + finishSz = min(PEM_LINE_LEN, bufferEnd - finish); + newline = XSTRNSTR(finish, "\r", finishSz); if (XMEMCPY(info->name, start, finish - start) == NULL) return SSL_FATAL_ERROR; @@ -4175,7 +4181,7 @@ int PemToDer(const unsigned char* buff, long longSz, int type, if (XMEMCPY(info->iv, finish + 1, sizeof(info->iv)) == NULL) return SSL_FATAL_ERROR; - if (!newline) newline = XSTRNSTR(finish, "\n", PEM_LINE_LEN); + if (!newline) newline = XSTRNSTR(finish, "\n", finishSz); if (newline && (newline > finish)) { info->ivSz = (word32)(newline - (finish + 1)); info->set = 1; From 17936d65e032f256a790752717d446a8de09b8e5 Mon Sep 17 00:00:00 2001 From: Levi Rak Date: Fri, 16 Jun 2017 12:27:59 -0600 Subject: [PATCH 2/4] please Jenkins + a bit of cleanup --- src/ssl.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 200106f4d..c7ee46fc9 100755 --- a/src/ssl.c +++ b/src/ssl.c @@ -4151,28 +4151,32 @@ int PemToDer(const unsigned char* buff, long longSz, int type, #if defined(OPENSSL_EXTRA) || defined(HAVE_WEBSERVER) { /* remove encrypted header if there */ - char encHeader[] = "Proc-Type"; - unsigned int headerEndSz = min(PEM_LINE_LEN, bufferEnd - headerEnd); - char* line = XSTRNSTR(headerEnd, encHeader, headerEndSz); - unsigned int lineSz = min(PEM_LINE_LEN, bufferEnd - line); - if (line) { - char* newline; - char* finish; - char* start = XSTRNSTR(line, "DES", lineSz); - unsigned int finishSz; - unsigned int startSz; + char encHeader[] = "Proc-Type"; + word32 headerEndSz = min((word32)((size_t)(bufferEnd - headerEnd)), + PEM_LINE_LEN); + char* line = XSTRNSTR(headerEnd, encHeader, headerEndSz); + if (line != NULL) { + word32 lineSz = min((word32)((size_t)(bufferEnd - line)), + PEM_LINE_LEN); + char* newline; + char* finish; + word32 finishSz; + char* start = XSTRNSTR(line, "DES", lineSz); + word32 startSz; - if (!start) + if (start == NULL) start = XSTRNSTR(line, "AES", lineSz); - if (!start) return SSL_BAD_FILE; - if (!info) return SSL_BAD_FILE; + if (start == NULL) return SSL_BAD_FILE; + if (info == NULL) return SSL_BAD_FILE; - startSz = min(PEM_LINE_LEN, bufferEnd - start); + startSz = min((word32)((size_t)(bufferEnd - line)), + PEM_LINE_LEN); finish = XSTRNSTR(start, ",", startSz); - if (start && finish && (start < finish)) { - finishSz = min(PEM_LINE_LEN, bufferEnd - finish); + if ((start != NULL) && (finish != NULL) && (start < finish)) { + finishSz = min((word32)((size_t)(bufferEnd - finish)), + PEM_LINE_LEN); newline = XSTRNSTR(finish, "\r", finishSz); if (XMEMCPY(info->name, start, finish - start) == NULL) @@ -4181,8 +4185,9 @@ int PemToDer(const unsigned char* buff, long longSz, int type, if (XMEMCPY(info->iv, finish + 1, sizeof(info->iv)) == NULL) return SSL_FATAL_ERROR; - if (!newline) newline = XSTRNSTR(finish, "\n", finishSz); - if (newline && (newline > finish)) { + if (newline == NULL) + newline = XSTRNSTR(finish, "\n", finishSz); + if ((newline != NULL) && (newline > finish)) { info->ivSz = (word32)(newline - (finish + 1)); info->set = 1; } From 247388903b4dbeb1ac0beb9d00b4fbf43f09e208 Mon Sep 17 00:00:00 2001 From: Levi Rak Date: Wed, 21 Jun 2017 11:09:40 -0600 Subject: [PATCH 3/4] Remove double cast + move min() calls --- src/ssl.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index c7ee46fc9..a3bdb2973 100755 --- a/src/ssl.c +++ b/src/ssl.c @@ -4152,32 +4152,29 @@ int PemToDer(const unsigned char* buff, long longSz, int type, { /* remove encrypted header if there */ char encHeader[] = "Proc-Type"; - word32 headerEndSz = min((word32)((size_t)(bufferEnd - headerEnd)), - PEM_LINE_LEN); - char* line = XSTRNSTR(headerEnd, encHeader, headerEndSz); + word32 headerEndSz = (word32)(bufferEnd - headerEnd); + char* line = XSTRNSTR(headerEnd, encHeader, min(headerEndSz, + PEM_LINE_LEN)); if (line != NULL) { - word32 lineSz = min((word32)((size_t)(bufferEnd - line)), - PEM_LINE_LEN); char* newline; char* finish; word32 finishSz; - char* start = XSTRNSTR(line, "DES", lineSz); word32 startSz; + word32 lineSz = (word32)(bufferEnd - line); + char* start = XSTRNSTR(line, "DES", min(lineSz, PEM_LINE_LEN)); if (start == NULL) - start = XSTRNSTR(line, "AES", lineSz); + start = XSTRNSTR(line, "AES", min(lineSz, PEM_LINE_LEN)); if (start == NULL) return SSL_BAD_FILE; if (info == NULL) return SSL_BAD_FILE; - startSz = min((word32)((size_t)(bufferEnd - line)), - PEM_LINE_LEN); - finish = XSTRNSTR(start, ",", startSz); + startSz = (word32)(bufferEnd - start); + finish = XSTRNSTR(start, ",", min((word32)startSz, PEM_LINE_LEN)); if ((start != NULL) && (finish != NULL) && (start < finish)) { - finishSz = min((word32)((size_t)(bufferEnd - finish)), - PEM_LINE_LEN); - newline = XSTRNSTR(finish, "\r", finishSz); + finishSz = (word32)(bufferEnd - finish); + newline = XSTRNSTR(finish, "\r", min(finishSz, PEM_LINE_LEN)); if (XMEMCPY(info->name, start, finish - start) == NULL) return SSL_FATAL_ERROR; @@ -4186,7 +4183,8 @@ int PemToDer(const unsigned char* buff, long longSz, int type, return SSL_FATAL_ERROR; if (newline == NULL) - newline = XSTRNSTR(finish, "\n", finishSz); + newline = XSTRNSTR(finish, "\n", min(finishSz, + PEM_LINE_LEN)); if ((newline != NULL) && (newline > finish)) { info->ivSz = (word32)(newline - (finish + 1)); info->set = 1; From a37808b32cb8947fd4d44ba969d8c6451148c64d Mon Sep 17 00:00:00 2001 From: Levi Rak Date: Wed, 21 Jun 2017 17:14:20 -0600 Subject: [PATCH 4/4] Sanity checkes added --- src/ssl.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index a3bdb2973..2615b556f 100755 --- a/src/ssl.c +++ b/src/ssl.c @@ -4156,23 +4156,39 @@ int PemToDer(const unsigned char* buff, long longSz, int type, char* line = XSTRNSTR(headerEnd, encHeader, min(headerEndSz, PEM_LINE_LEN)); if (line != NULL) { - char* newline; + word32 lineSz; char* finish; word32 finishSz; + char* start; word32 startSz; - word32 lineSz = (word32)(bufferEnd - line); - char* start = XSTRNSTR(line, "DES", min(lineSz, PEM_LINE_LEN)); + char* newline; - if (start == NULL) + if (line >= bufferEnd) { + return SSL_BAD_FILE; + } + + lineSz = (word32)(bufferEnd - line); + start = XSTRNSTR(line, "DES", min(lineSz, PEM_LINE_LEN)); + + if (start == NULL) { start = XSTRNSTR(line, "AES", min(lineSz, PEM_LINE_LEN)); + } if (start == NULL) return SSL_BAD_FILE; if (info == NULL) return SSL_BAD_FILE; + if (start >= bufferEnd) { + return SSL_BAD_FILE; + } + startSz = (word32)(bufferEnd - start); - finish = XSTRNSTR(start, ",", min((word32)startSz, PEM_LINE_LEN)); + finish = XSTRNSTR(start, ",", min(startSz, PEM_LINE_LEN)); if ((start != NULL) && (finish != NULL) && (start < finish)) { + if (finish >= bufferEnd) { + return SSL_BAD_FILE; + } + finishSz = (word32)(bufferEnd - finish); newline = XSTRNSTR(finish, "\r", min(finishSz, PEM_LINE_LEN));