[OpenSSL-1.0.2] TNF local patch - use strl{cat,cpy}, snprintf instead of unsafe function.

[Takehiko NOZAKI]

original commit message: TBD

[Takehiko NOZAKI]

all diffs are kind of “replace secure function” silly myth.

rewrite the code from:

strcpy(buf, str);

to:

strlcpy(buf, sizeof(buf), str);

is not considered secure nowadays.

you must check string truncation error too.

if (strlcpy(buf, sizeof(buf), str) >= sizeof(buf)) {
  /* do string truncate error handle */
}

[Takehiko NOZAKI]

following diff:

diff --exclude=CVS -upNr openssl-1.0.2k/apps/ca.c src/crypto/external/bsd/openssl/dist/apps/ca.c
--- openssl-1.0.2k/apps/ca.c    2017-01-26 22:22:03.000000000 +0900
+++ src/crypto/external/bsd/openssl/dist/apps/ca.c      2017-01-28 08:16:20.000000000 +0900
@@ -1235,7 +1235,7 @@ int MAIN(int argc, char **argv)
                 goto err;
             }

-            strcpy(buf[2], outdir);
+            strlcpy(buf[2], outdir, sizeof(buf[2]));

 #ifndef OPENSSL_SYS_VMS
             BUF_strlcat(buf[2], "/", sizeof(buf[2]));

this is original code:

 318 #undef BSIZE
 319 #define BSIZE 256
 320 MS_STATIC char buf[3][BSIZE];

1232
1233             if (strlen(outdir) >= (size_t)(j ? BSIZE - j * 2 - 6 : BSIZE - 8)) {
1234                 BIO_printf(bio_err, "certificate file name too long\n");
1235                 goto err;
1236             }
1237
1238             strcpy(buf[2], outdir);

line 1233, strlen(outdif) is smaller than BSIZE, so using strlcpy(3) is meaningless.

[Takehiko NOZAKI]

following diff:

diff --exclude=CVS -upNr openssl-1.0.2k/crypto/bio/bss_file.c src/crypto/external/bsd/openssl/dist/crypto/bio/bss_file.c
--- openssl-1.0.2k/crypto/bio/bss_file.c        2017-01-26 22:22:03.000000000 +0900
+++ src/crypto/external/bsd/openssl/dist/crypto/bio/bss_file.c  2016-10-15 01:23:18.000000000 +0900
@@ -387,15 +387,15 @@ static long MS_CALLBACK file_ctrl(BIO *b
         }
 #  if defined(OPENSSL_SYS_MSDOS) || defined(OPENSSL_SYS_WINDOWS) || defined(OPENSSL_SYS_OS2) || defined(OPENSSL_SYS_WIN32_CYGWIN)
         if (!(num & BIO_FP_TEXT))
-            strcat(p, "b");
+            strlcat(p, "b", sizeof(p));
         else
-            strcat(p, "t");
+            strlcat(p, "t", sizeof(p));
 #  endif
 #  if defined(OPENSSL_SYS_NETWARE)
         if (!(num & BIO_FP_TEXT))
-            strcat(p, "b");
+            strlcat(p, "b", sizeof(p));
         else
-            strcat(p, "t");
+            strlcat(p, "t", sizeof(p));
 #  endif
         fp = file_fopen(ptr, p);
         if (fp == NULL) {

this ifdef is for MSDOS/Windows/Cygwin and NETWARE.

completely meaningless for me.

[Takehiko NOZAKI]

following diff:

diff --exclude=CVS -upNr openssl-1.0.2k/crypto/bio/bss_log.c src/crypto/external/bsd/openssl/dist/crypto/bio/bss_log.c
--- openssl-1.0.2k/crypto/bio/bss_log.c 2017-01-26 22:22:03.000000000 +0900
+++ src/crypto/external/bsd/openssl/dist/crypto/bio/bss_log.c   2015-03-23 19:22:46.000000000 +0900
@@ -322,7 +322,7 @@ static void xsyslog(BIO *bp, int priorit
         break;
     }

-    sprintf(pidbuf, "[%u] ", GetCurrentProcessId());
+    snprintf(pidbuf, sizeof(pidbuf), "[%u] ", GetCurrentProcessId());
     lpszStrings[0] = pidbuf;
     lpszStrings[1] = string;

this is original code:

# if defined(OPENSSL_SYS_WIN32)

static void xopenlog(BIO *bp, char *name, int level)
{
    if (check_winnt())
        bp->ptr = RegisterEventSourceA(NULL, name);
    else
        bp->ptr = NULL;
}

static void xsyslog(BIO *bp, int priority, const char *string)
{
...

this function is for Win32, it’s useless for me.

[Takehiko NOZAKI]

following diff:

diff --exclude=CVS -upNr openssl-1.0.2k/crypto/dh/dhtest.c src/crypto/external/bsd/openssl/dist/crypto/dh/dhtest.c
--- openssl-1.0.2k/crypto/dh/dhtest.c   2017-01-26 22:22:03.000000000 +0900
+++ src/crypto/external/bsd/openssl/dist/crypto/dh/dhtest.c     2016-10-15 01:23:18.000000000 +0900
@@ -181,7 +181,7 @@ int main(int argc, char *argv[])

     BIO_puts(out, "key1 =");
     for (i = 0; i < aout; i++) {
-        sprintf(buf, "%02X", abuf[i]);
+        snprintf(buf, sizeof(buf), "%02X",abuf[i]);
         BIO_puts(out, buf);
     }
     BIO_puts(out, "\n");
@@ -192,7 +192,7 @@ int main(int argc, char *argv[])

     BIO_puts(out, "key2 =");
     for (i = 0; i < bout; i++) {
-        sprintf(buf, "%02X", bbuf[i]);
+        snprintf(buf, sizeof(buf), "%02X",bbuf[i]);
         BIO_puts(out, buf);
     }
     BIO_puts(out, "\n");

this is original code:

int main(int argc, char *argv[])
{
    BN_GENCB _cb;
    DH *a;
    DH *b = NULL;
    char buf[12];
...

buf size is enough to store the result of %02X conversion, no harm of buffer overflow.

[Takehiko NOZAKI]

useless, NAK.