From 2fb269a5b0cb908b56c9a06b5af798d6cfab94e0 Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Fri, 18 May 2018 00:00:00 -0500 Subject: [PATCH] top: Fix out-of-bounds read/write in show_... REVERTED I'm reverting this patch to prepare for some alternate solution. In that solution I will address point #1 but point #2 is based on a wrong assumption. There will be no binary data ever found in the 'glob' passed to this show_special() function. It is now always simple text. ------------------------------------------------ original commit message 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. ------------------------------------------------------------------------ Reference(s): . original qualys patch 0116-top-Fix-out-of-bounds-read-write-in-show_special.patch commit ed8f6d9cc68fbadb26ee3009a3017b3e3ea63f28 Signed-off-by: Jim Warner --- top/top.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/top/top.c b/top/top.c index 5088a8d6..54920a72 100644 --- a/top/top.c +++ b/top/top.c @@ -963,6 +963,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 + *(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'; @@ -971,8 +973,6 @@ static void show_special (int interact, const char *glob) { rp = scat(rp, tmp); 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 @@ -980,7 +980,6 @@ 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);