From ffc65b382753a1d61b8f43e4d86f5415b7bfb415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Sat, 12 Sep 2020 19:05:56 +0200 Subject: [PATCH] Reorder check to avoid crash on invalid process field setting If using a setting from a different development version with an unsupported process field, first dereferencing Process_fields[id] yields to a crash: ================================================================= ==19530==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000612800 at pc 0x000000521d1a bp 0x7ffec47a5ff0 sp 0x7ffec47a5fe8 READ of size 8 at 0x000000612800 thread T0 #0 0x521d19 in readFields .htop/Settings.c:107:40 #1 0x51d117 in Settings_read .htop/Settings.c:141:10 #2 0x51c0c4 in Settings_new .htop/Settings.c:382:12 #3 0x4eafe2 in main .htop/htop.c:220:25 #4 0x7fa450570cc9 in __libc_start_main csu/../csu/libc-start.c:308:16 #5 0x427a59 in _start (.htop/htop+0x427a59) 0x000000612800 is located 0 bytes to the right of global variable 'Process_fields' defined in 'linux/LinuxProcess.c:24:18' (0x6118a0) of size 3936 SUMMARY: AddressSanitizer: global-buffer-overflow .htop/Settings.c:107:40 in readFields Shadow bytes around the buggy address: 0x0000800ba4b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800ba4c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800ba4d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800ba4e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0000800ba4f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0000800ba500:[f9]f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x0000800ba510: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x0000800ba520: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x0000800ba530: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x0000800ba540: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x0000800ba550: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==19530==ABORTING --- Settings.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Settings.c b/Settings.c index d6ef53b9..0aac4796 100644 --- a/Settings.c +++ b/Settings.c @@ -100,7 +100,7 @@ static void readFields(ProcessField* fields, int* flags, const char* line) { for (j = 0, i = 0; i < Platform_numberOfFields && ids[i]; i++) { // This "+1" is for compatibility with the older enum format. int id = atoi(ids[i]) + 1; - if (id > 0 && Process_fields[id].name && id < Platform_numberOfFields) { + if (id > 0 && id < Platform_numberOfFields && Process_fields[id].name) { fields[j] = id; *flags |= Process_fields[id].flags; j++;