]> granicus.if.org Git - procps-ng/commitdiff
top: address 'show_special()' o-o-b read/write concern
authorJim Warner <james.warner@comcast.net>
Fri, 18 May 2018 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@enc.com.au>
Sat, 19 May 2018 11:45:36 +0000 (21:45 +1000)
This patch addresses a potential (but unlikely) buffer
overflow by reducing, if necessary, a memcpy length by
3 bytes to provide for an eol '\0' and 2 unused buffer
positions which also might receive the '\0' character.

[ note to future analysis tool: just because you see ]
[ binary data being manipulated in the routine, that ]
[ doesn't mean such function was passed binary data! ]

Reference(s):
. original qualys patch
0116-top-Fix-out-of-bounds-read-write-in-show_special.patch
commit ed8f6d9cc68fbadb26ee3009a3017b3e3ea63f28

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

index 54920a7207ad2f7a6cb1f27d7b836137a6aa5e51..935b8f546404ca8b15ba3cf0e82d20c9f7e0f88f 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -928,7 +928,7 @@ static void show_special (int interact, const char *glob) {
   /* note: the following is for documentation only,
            the real captab is now found in a group's WIN_t !
      +------------------------------------------------------+
-     | char *captab[] = {                 :   Cap's/Delim's |
+     | char *captab[] = {                 :   Cap's = Index |
      |   Cap_norm, Cap_norm,              =   \000, \001,   |
      |   cap_bold, capclr_sum,            =   \002, \003,   |
      |   capclr_msg, capclr_pmt,          =   \004, \005,   |
@@ -938,9 +938,16 @@ static void show_special (int interact, const char *glob) {
      +------------------------------------------------------+ */
   /* ( Pssst, after adding the termcap transitions, row may )
      ( exceed 300+ bytes, even in an 80x24 terminal window! )
-     ( And if we're no longer guaranteed lines created only )
-     ( by top, we'll need larger buffs plus some protection )
-     ( against overrunning them with this 'lin_end - glob'. ) */
+     ( Shown here are the former buffer size specifications )
+     ( char tmp[SMLBUFSIZ], lin[MEDBUFSIZ], row[LRGBUFSIZ]. )
+     ( So now we use larger buffers and a little protection )
+     ( against overrunning them with this 'lin_end - glob'. )
+
+     ( That was uncovered during 'Inspect' development when )
+     ( this guy was being considered for a supporting role. )
+     ( However, such an approach was abandoned. As a result )
+     ( this function is called only with a glob under top's )
+     ( control and never containing any 'raw/binary' chars! ) */
    char tmp[LRGBUFSIZ], lin[LRGBUFSIZ], row[ROWMAXSIZ];
    char *rp, *lin_end, *sub_beg, *sub_end;
    int room;
@@ -948,7 +955,7 @@ static void show_special (int interact, const char *glob) {
    // handle multiple lines passed in a bunch
    while ((lin_end = strchr(glob, '\n'))) {
      #define myMIN(a,b) (((a) < (b)) ? (a) : (b))
-      size_t lessor = myMIN((size_t)(lin_end - glob), sizeof(lin) -1);
+      size_t lessor = myMIN((size_t)(lin_end - glob), sizeof(lin) -3);
 
       // create a local copy we can extend and otherwise abuse
       memcpy(lin, glob, lessor);
@@ -963,6 +970,8 @@ static void show_special (int interact, const char *glob) {
          if ('~' == ch) ch = *(sub_end + 1) - '0';
          switch (ch) {
             case 0:                    // no end delim, captab makes normal
+               // only possible when '\n' was NOT preceeded with a '~#' sequence
+               // ( '~1' thru '~8' is valid range, '~0' is never actually used )
                *(sub_end + 1) = '\0';  // extend str end, then fall through
                *(sub_end + 2) = '\0';  // ( +1 optimization for usual path )
             case 1: case 2: case 3: case 4: