]> granicus.if.org Git - procps-ng/commitdiff
top: Fix out-of-bounds read/write in show_special().
authorQualys Security Advisory <qsa@qualys.com>
Thu, 1 Jan 1970 00:00:00 +0000 (00:00 +0000)
committerCraig Small <csmall@enc.com.au>
Fri, 18 May 2018 21:33:15 +0000 (07:33 +1000)
This patch fixes two problems:

1/ In the switch case 0, if sub_end is at the very end of lin[], the two
null-byte writes are off-by-two (a stack-based buffer overflow). Replace
this end-of-string "emulation" with an equivalent test on ch (and then
goto/break out of the loop).

2/ "sub_end += 2" jumps over the null-byte terminator in lin[] if the
line contains a raw (without a tilde) \001-\010 character. Detect such a
null-byte terminator and goto/break out of the loop.

Note: in the case of a raw \001-\010 character, the character at
"sub_end + 1" is never processed (it is skipped/jumped over); this is
not a security problem anymore (since 2/ was fixed), so we decided not
to change this behavior, for backward-compatibility.

top/top.c

index 018c8bb718f3a33ad6440561b338ddee76c6c2ce..7ff52e97e94d5657f042fdc38668eb0df61b0ae3 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -972,8 +972,6 @@ 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
-               *(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:
             case 5: case 6: case 7: case 8:
                *sub_end = '\0';
@@ -982,6 +980,8 @@ static void show_special (int interact, const char *glob) {
                rp = scat(rp, tmp, row, sizeof(row));
                room -= (sub_end - sub_beg);
                room += utf8_delta(sub_beg);
+               if (!ch) goto done_substrings;
+               if (!*(sub_end + 1)) goto done_substrings;
                sub_beg = (sub_end += 2);
                break;
             default:                   // nothin' special, just text
@@ -989,6 +989,7 @@ static void show_special (int interact, const char *glob) {
          }
          if (0 >= room) break;         // skip substrings that won't fit
       }
+done_substrings:
 
       if (interact) PUTT("%s%s\n", row, Cap_clr_eol);
       else PUFF("%s%s\n", row, Caps_endline);