]> granicus.if.org Git - procps-ng/commitdiff
0048-proc/escape.c: Make sure all escape*() arguments are safe.
authorQualys Security Advisory <qsa@qualys.com>
Thu, 1 Jan 1970 00:00:00 +0000 (00:00 +0000)
committerCraig Small <csmall@enc.com.au>
Sat, 9 Jun 2018 11:35:19 +0000 (21:35 +1000)
The SECURE_ESCAPE_ARGS() macro solves several potential problems
(although we found no problematic calls to the escape*() functions in
procps's code-base, but had to thoroughly review every call; and this is
library code):

1/ off-by-one overflows if the size of the destination buffer is 0;

2/ buffer overflows if this size (or "maxroom") is negative;

3/ integer overflows (for example, "*maxcells+1");

4/ always null-terminate the destination buffer (unless its size is 0).

---------------------------- adapted for newlib branch
. the escape.c now has just a single exported function
. thus SECURE_ESCAPE_ARGS() is needed in only 2 places
. unlike that original patch, macro is executed 1 time
( not like 'escape_command' calling 'escape_strlist' )
( which might then call 'escape_str' multiple times! )

Signed-off-by: Jim Warner <james.warner@comcast.net>
proc/escape.c

index 18b46fde594f42a46eb8f4152ca32406622c8fcc..62123530fe57bf32c6100cb4803e5f60da237087 100644 (file)
@@ -17,6 +17,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <limits.h>
 #include <stdio.h>
 #include <sys/types.h>
 #include <string.h>
 # include <langinfo.h>
 #endif
 
+#define SECURE_ESCAPE_ARGS(dst, bytes, cells) do { \
+  if ((bytes) <= 0) return 0; \
+  *(dst) = '\0'; \
+  if ((bytes) >= INT_MAX) return 0; \
+  if ((cells) >= INT_MAX) return 0; \
+  if ((cells) <= 0) return 0; \
+} while (0)
+
+
 #if (__GNU_LIBRARY__ >= 6) && (!defined(__UCLIBC__) || defined(__UCLIBC_HAS_WCHAR__))
 static int escape_str_utf8(char *restrict dst, const char *restrict src, int bufsize, int *maxcells){
   int my_cells = 0;
   int my_bytes = 0;
   mbstate_t s;
 
+  SECURE_ESCAPE_ARGS(dst, bufsize, *maxcells);
+
   memset(&s, 0, sizeof (s));
 
   for(;;) {
@@ -147,6 +159,8 @@ int escape_str(char *restrict dst, const char *restrict src, int bufsize, int *m
   }
 #endif
 
+  SECURE_ESCAPE_ARGS(dst, bufsize, *maxcells);
+
   if(bufsize > *maxcells+1) bufsize=*maxcells+1; // FIXME: assumes 8-bit locale
 
   for(;;){