Make snprintf always terminate the output buffer

Previously, the snprintf function would not terminate the output
buffer if it had already filled the entire buffer with
characters. This would lead to the creation of unterminated strings
which could result in buffer overreads and/or information
disclosures.

This commit fixes that issue, making it conform to the C99 standard
(section 7.19.6.5).

Also addresses a buffer underflow that could occur when the source
string to an sprintf call contains too many \b characters.

Change-Id: Ie516b1c7d74d37cc5f48fe03693f096fe4bd6c02
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/84349
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: William G Hoffa <wghoffa@us.ibm.com>

1 files changed, 24 insertions, 6 deletions

diff --git a/src/lib/stdio.C b/src/lib/stdio.C
index d61b48cf9..9ed438f78 100644
--- a/src/lib/stdio.C
+++ b/src/lib/stdio.C
@@ -5,7 +5,9 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* COPYRIGHT International Business Machines Corp. 2011,2014              */
+/* Contributors Listed Below - COPYRIGHT 2011,2019                        */
+/* [+] International Business Machines Corp.                              */
+/*                                                                        */
 /*                                                                        */
 /* Licensed under the Apache License, Version 2.0 (the "License");        */
 /* you may not use this file except in compliance with the License.       */
@@ -31,7 +33,10 @@ class SprintfBuffer : public Util::ConsoleBufferInterface
         {
             if ('\b' == c)
             {
-                iv_pos--;
+                if (iv_pos > 0)
+                {
+                    iv_pos--;
+                }
             }
             else if (iv_pos < iv_size)
             {
@@ -44,6 +49,19 @@ class SprintfBuffer : public Util::ConsoleBufferInterface
             return c;
         }
 
+        void nullTerminate()
+        {
+            if (iv_size > 0)
+            {
+                if (iv_pos >= iv_size)
+                {
+                    iv_pos = iv_size - 1;
+                }
+
+                putc('\0');
+            }
+        }
+
         explicit SprintfBuffer(char* buf, size_t size = UINT64_MAX) :
             iv_pos(0), iv_size(size), iv_buffer(buf) {};
 
@@ -66,7 +84,7 @@ int sprintf(char *str, const char * format, ...)
     size_t count = vasprintf(console, format, args);
 
     va_end(args);
-    console.putc('\0');
+    console.nullTerminate();
     return count;
 }
 
@@ -81,7 +99,7 @@ int snprintf(char *str, size_t size, const char * format, ...)
     size_t count = vasprintf(console, format, args);
 
     va_end(args);
-    console.putc('\0');
+    console.nullTerminate();
     return count;
 }
 
@@ -92,7 +110,7 @@ int vsprintf(char *str, const char * format, va_list args)
     SprintfBuffer console(str);
     size_t count = vasprintf(console, format, args);
 
-    console.putc('\0');
+    console.nullTerminate();
     return count;
 }
 
@@ -101,6 +119,6 @@ int vsnprintf(char *str, size_t size, const char * format, va_list args)
     SprintfBuffer console(str, size);
     size_t count = vasprintf(console, format, args);
 
-    console.putc('\0');
+    console.nullTerminate();
     return count;
 }