|
Message-ID: <20240130183915.GB16546@localhost.localdomain> Date: Tue, 30 Jan 2024 18:39:37 +0000 From: Qualys Security Advisory <qsa@...lys.com> To: "oss-security@...ts.openwall.com" <oss-security@...ts.openwall.com> Subject: Out-of-bounds read & write in the glibc's qsort() Qualys Security Advisory For the algorithm lovers: Nontransitive comparison functions lead to out-of-bounds read & write in glibc's qsort() ======================================================================== Contents ======================================================================== Summary Background Experiments Analysis Patch Discussion Acknowledgments Timeline CUT MY LIST IN TWO PIECES THAT'S HOW YOU START QUICK SORT -- https://twitter.com/QuinnyPig/status/1710447650112438710 ======================================================================== Summary ======================================================================== We discovered a memory corruption in the glibc's qsort() function, due to a missing bounds check. To be vulnerable, a program must call qsort() with a nontransitive comparison function (a function cmp(int a, int b) that returns (a - b), for example) and with a large number of attacker- controlled elements (to cause a malloc() failure inside qsort()). We have not tried to find such a vulnerable program in the real world. All glibc versions from at least September 1992 (glibc 1.04) to the current release (glibc 2.38) are affected, but the glibc's developers have independently discovered and patched this memory corruption in the master branch (commit b9390ba, "stdlib: Fix array bounds protection in insertion sort phase of qsort") during a recent refactoring of qsort(). About our advisory, the glibc security team issues the following statement: ------------------------------------------------------------------------ This memory corruption in the GNU C Library through the qsort function is invoked by an application passing a non-transitive comparison function, which is undefined according to POSIX and ISO C standards. As a result, we are of the opinion that the resulting CVE, if any, should be assigned to any such calling applications and subsequently fixed by passing a valid comparison function to qsort and not to glibc. We however acknowledge that this is a quality of implementation issue and we fixed this in a recent refactor of qsort. We would like to thank Qualys for sharing their findings and helping us validate our recent changes to qsort. ------------------------------------------------------------------------ ======================================================================== Background ======================================================================== While browsing through Postfix's HISTORY file, we stumbled across a puzzling entry from February 2002: ------------------------------------------------------------------------ Bugfix: make all recipient comparisons transitive, because Solaris qsort() causes SIGSEGV errors otherwise. Victor Duchovni, Morgan Stanley. File: *qmgr/qmgr_message.c. ------------------------------------------------------------------------ Segmentation faults in qsort()? Transitive comparison functions? As explained in the manual page for qsort(), "The comparison function must return an integer less than, equal to, or greater than zero if the first argument is considered to be respectively less than, equal to, or greater than the second." Of course, such a comparison function cmp() must be transitive: - if a < b (i.e., if cmp(pointer_to(a), pointer_to(b)) < 0); - and if b < c (i.e., if cmp(pointer_to(b), pointer_to(c)) < 0); - then necessarily a < c (i.e., cmp(pointer_to(a), pointer_to(c)) < 0). For example, the following comparison function (which compares integers) is transitive (and perfectly correct): ------------------------------------------------------------------------ int cmp(const void * const pa, const void * const pb) { const int a = *(const int *)pa; const int b = *(const int *)pb; if (a > b) return +1; if (a < b) return -1; return 0; } ------------------------------------------------------------------------ A shorter and more efficient version of this comparison function could simply "return (a > b) - (a < b);" and still be transitive and perfectly correct: - if a > b, it returns 1 - 0 = +1; - if a < b, it returns 0 - 1 = -1; - if a = b, it returns 0 - 0 = 0. The question, then, is: how can a comparison function be nontransitive? A comparison function cmp() is nontransitive if there exist a, b, and c such that: - a < b (because cmp(pointer_to(a), pointer_to(b)) < 0); - b < c (because cmp(pointer_to(b), pointer_to(c)) < 0); - but a >= c (because cmp(pointer_to(a), pointer_to(c)) >= 0 by mistake). Although the following comparison function seems correct at first, it is in fact nontransitive, because the subtraction in "return (a - b);" is prone to integer overflows: ------------------------------------------------------------------------ int cmp(const void * const pa, const void * const pb) { const int a = *(const int *)pa; const int b = *(const int *)pb; return (a - b); } ------------------------------------------------------------------------ For example, if a = INT_MIN, b = 0, and c = INT_MAX, then: - a < b (because cmp(pointer_to(a), pointer_to(b)) returns INT_MIN - 0, which is correctly negative); - b < c (because cmp(pointer_to(b), pointer_to(c)) returns 0 - INT_MAX, which is also correctly negative); - but a > c by mistake (because cmp(pointer_to(a), pointer_to(c)) returns INT_MIN - INT_MAX = +1, which is incorrectly positive because this subtraction overflows). Unfortunately, such nontransitive comparison functions are extremely common, as discussed in this excellent blog post from Ted Unangst: https://flak.tedunangst.com/post/subtraction-is-not-comparison and as hinted at in OpenBSD's manual page for qsort(): "It is almost always an error to use subtraction to compute the return value of the comparison function." Fortunately, when passed to a robust qsort() implementation, these nontransitive comparison functions should (at the worst) result in an incorrectly sorted array; certainly not in a memory corruption. However, the aforementioned entry from Postfix's HISTORY file suggests that not all qsort() implementations are robust. ======================================================================== Experiments ======================================================================== We therefore decided to assess the robustness of the glibc's qsort() implementation, by calling it with a nontransitive comparison function: ------------------------------------------------------------------------ 1 #include <limits.h> 2 #include <stdlib.h> 3 #include <sys/time.h> 4 5 static int 6 cmp(const void * const pa, const void * const pb) 7 { 8 const int a = *(const int *)pa; 9 const int b = *(const int *)pb; 10 return (a - b); 11 } 12 13 int 14 main(const int argc, const char * const argv[]) 15 { 16 if (argc != 2) return __LINE__; 17 const size_t nmemb = strtoul(argv[1], NULL, 0); 18 if (nmemb <= 0 || nmemb >= (1<<28)) return __LINE__; 19 20 int * const pcanary1 = calloc(1 + nmemb + 1, sizeof(int)); 21 if (!pcanary1) return __LINE__; 22 int * const array = pcanary1 + 1; 23 int * const pcanary2 = array + nmemb; 24 25 struct timeval tv; 26 if (gettimeofday(&tv, NULL)) return __LINE__; 27 srandom((tv.tv_sec << 16) ^ tv.tv_usec); 28 29 const int canary1 = *pcanary1 = (random() << 16) ^ random(); 30 const int canary2 = *pcanary2 = (random() << 16) ^ random(); 31 array[random() % nmemb] = INT_MIN; 32 33 qsort(array, nmemb, sizeof(int), cmp); 34 if (*pcanary1 != canary1) abort(); 35 if (*pcanary2 != canary2) abort(); 36 return 0; 37 } ------------------------------------------------------------------------ - at lines 5-11, cmp() is the nontransitive comparison function introduced in the previous "Background" section; - at lines 16-18, the number of elements to be sorted (simple integers) is read from the command line; - at lines 20-23, the array of elements to be sorted is calloc()ated, along with a canary element below this array, and a canary element above this array; - at lines 29-30, these two canary elements are randomized, and copied to the stack for later comparison; - at line 31, one random element of the array is initialized to INT_MIN (all other elements are initialized to 0 by calloc()); - at line 33, the elements of this array are sorted by qsort(); - at lines 34-35, the two canary elements (below and above the sorted array) are checked against their stack copies, and if they differ (an out-of-bounds write in qsort()), abort() is called. We chose the array elements a = INT_MIN and b = 0 because they directly exhibit the problematic behavior of this cmp() function: - a < b, because cmp(pointer_to(a), pointer_to(b)) returns INT_MIN - 0, which is correctly negative; - but b < a by mistake, because cmp(pointer_to(b), pointer_to(a)) returns 0 - INT_MIN = INT_MIN (the "Leblancian Paradox"), which is incorrectly negative (because this subtraction overflows). We then executed our test program in a loop, on Fedora 39 (which uses the latest glibc version, 2.38): ------------------------------------------------------------------------ $ while true; do n=$((RANDOM*64+RANDOM+1)); ./qsort $n; done ------------------------------------------------------------------------ Unsurprisingly, nothing happened: our program did not crash or abort(). While this loop was still running (and not crashing), we started to read the glibc's qsort() implementation; to our great surprise, we discovered that the glibc's qsort() is not, in fact, a quick sort by default, but a merge sort (in stdlib/msort.c). Most likely, merge sort was chosen over quick sort to avoid quick sort's worst-case performance, which is O(n^2); on the other hand, merge sort's worst-case performance is O(n*log(n)). But merge sort suffers from one major drawback: it does not sort in-place -- it malloc()ates a copy of the array of elements to be sorted. As a result, if this array is very large (lines 212-217), or if this malloc() fails (lines 219-229), then the glibc's qsort() falls back to a quick sort (in stdlib/qsort.c), because quick sort does sort in-place: ------------------------------------------------------------------------ 163 void 164 __qsort_r (void *b, size_t n, size_t s, __compar_d_fn_t cmp, void *arg) 165 { 166 size_t size = n * s; ... 170 /* For large object sizes use indirect sorting. */ 171 if (s > 32) 172 size = 2 * n * sizeof (void *) + s; 173 174 if (size < 1024) 175 /* The temporary array is small, so put it on the stack. */ 176 p.t = __alloca (size); 177 else 178 { ... 212 /* If the memory requirements are too high don't allocate memory. */ 213 if (size / pagesize > (size_t) phys_pages) 214 { 215 _quicksort (b, n, s, cmp, arg); 216 return; 217 } 218 219 /* It's somewhat large, so malloc it. */ 220 int save = errno; 221 tmp = malloc (size); 222 __set_errno (save); 223 if (tmp == NULL) 224 { 225 /* Couldn't get space, so use the slower algorithm 226 that doesn't need a temporary array. */ 227 _quicksort (b, n, s, cmp, arg); 228 return; 229 } 230 p.t = tmp; 231 } ... 299 } ------------------------------------------------------------------------ We therefore decided to assess the robustness of the glibc's quick sort (instead of its merge sort, which was clearly not crashing), by forcing qsort() to call _quicksort(). Locally, forcing the malloc() at line 221 to fail is very easy: we simply execute our program with a low RLIMIT_AS ("The maximum size of the process's virtual memory", man setrlimit); and this works even when executing a SUID-root program. So we executed our program in the following loop instead: ------------------------------------------------------------------------ $ while true; do n=$((RANDOM*64+RANDOM+1)); prlimit --as=$((n*4/2*3)) ./qsort $n; done Aborted (core dumped) Aborted (core dumped) Aborted (core dumped) ... ------------------------------------------------------------------------ Incredibly, we almost immediately observed crashes of our test program: calls to abort(), because one of our canary elements (below or above the sorted array) was overwritten (i.e., an out-of-bounds write in qsort()). To understand these crashes, we examined one of them in gdb: ------------------------------------------------------------------------ $ gdb prlimit (gdb) run --as=8104854 ./qsort 1350809 Starting program: /usr/bin/prlimit --as=8104854 ./qsort 1350809 ... Program received signal SIGABRT, Aborted. __pthread_kill_implementation (threadid=<optimized out>, signo=signo@...ry=6, no_tid=no_tid@...ry=0) at pthread_kill.c:44 44 return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; (gdb) backtrace #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@...ry=6, no_tid=no_tid@...ry=0) at pthread_kill.c:44 #1 0x00007ffff7e698a3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78 #2 0x00007ffff7e178ee in __GI_raise (sig=sig@...ry=6) at ../sysdeps/posix/raise.c:26 #3 0x00007ffff7dff8ff in __GI_abort () at abort.c:79 #4 0x0000555555555334 in main (argc=2, argv=0x7fffffffe338) at qsort.c:34 (gdb) select-frame 4 (gdb) p/x canary1 $1 = 0xc6109e4c (gdb) p/x *pcanary1 $2 = 0x0 (gdb) x/xw pcanary1 - 2 0x7ffff78af008: 0x00528002 0x7ffff78af00c: 0x80000000 0x7ffff78af010: 0x00000000 0x7ffff78af014: 0xc6109e4c 0x7ffff78af018: 0x00000000 ------------------------------------------------------------------------ - at address 0x7ffff78af010 (pcanary1), the original value of the canary (0xc6109e4c) was overwritten with 0x0 -- an out-of-bounds write; - at address 0x7ffff78af00c (below pcanary1), the most significant word of an mchunk_size (heap metadata) was overwritten with 0x80000000 (INT_MIN) -- another out-of-bounds write; - at address 0x7ffff78af014 (above pcanary1), the first element of the array was overwritten with 0xc6109e4c (the original value of the canary), which was therefore read out-of-bounds beforehand (from pcanary1). ======================================================================== Analysis ======================================================================== To identify the root cause of these out-of-bounds memory accesses, we must analyze the implementation of the glibc's quick sort: ------------------------------------------------------------------------ 87 void 88 _quicksort (void *const pbase, size_t total_elems, size_t size, 89 __compar_d_fn_t cmp, void *arg) 90 { 91 char *base_ptr = (char *) pbase; ... 108 while (STACK_NOT_EMPTY) 109 { ... 193 } ... 206 char *tmp_ptr = base_ptr; ... 214 for (run_ptr = tmp_ptr + size; run_ptr <= thresh; run_ptr += size) 215 if ((*cmp) ((void *) run_ptr, (void *) tmp_ptr, arg) < 0) 216 tmp_ptr = run_ptr; 217 218 if (tmp_ptr != base_ptr) 219 SWAP (tmp_ptr, base_ptr, size); ... 223 run_ptr = base_ptr + size; 224 while ((run_ptr += size) <= end_ptr) 225 { 226 tmp_ptr = run_ptr - size; 227 while ((*cmp) ((void *) run_ptr, (void *) tmp_ptr, arg) < 0) 228 tmp_ptr -= size; ... 246 } ... 248 } ------------------------------------------------------------------------ - at lines 108-193, when quick sort's partitions become smaller than MAX_THRESH (4 elements), _quicksort() switches to a final insertion sort (at lines 206-246), which is faster than quick sort for small or mostly sorted arrays; - at lines 206-219, this insertion sort makes sure that the very first element of the array (base_ptr) is the smallest element of the array; - at lines 226-228, this first element acts as a natural barrier that prevents tmp_ptr from being decremented below the array (because if tmp_ptr reaches base_ptr, then necessarily cmp(run_ptr, tmp_ptr) >= 0 because tmp_ptr is base_ptr, the smallest element of the array); - unfortunately this does not hold true if cmp() is nontransitive, in which case cmp(run_ptr, tmp_ptr) can be < 0 even if tmp_ptr is base_ptr, so tmp_ptr can be decremented below the array, where out-of-bounds elements are read and overwritten. ======================================================================== Patch ======================================================================== To patch these out-of-bounds memory accesses in _quicksort(), a simple check "tmp_ptr > base_ptr &&" can be added in front of the cmp() call at line 227 (of course this does not magically result in a correctly sorted array if cmp() is nontransitive, but at least it does not result in a memory corruption anymore). In fact, while drafting this advisory, we discovered that such a check ("tmp_ptr != base_ptr &&") has already been added to the glibc's master branch (which will become glibc 2.39 in February 2024), by the following commit ("stdlib: Fix array bounds protection in insertion sort phase of qsort"): https://sourceware.org/git?p=glibc.git;a=commit;h=b9390ba93676c4b1e87e218af5e7e4bb596312ac Indeed, the glibc developers have recently refactored qsort() and replaced the merge sort (and its fallback to quick sort) with an introspective sort (a combination of quick sort, heap sort, and insertion sort): https://en.wikipedia.org/wiki/Introsort https://sourceware.org/pipermail/libc-alpha/2023-October/151907.html During this refactoring, the glibc developers have proactively discovered (and patched) the out-of-bounds memory accesses in the insertion sort (probably because these out-of-bounds memory accesses became directly exposed to the misbehavior of nontransitive comparison functions, instead of being safely hidden behind a malloc() failure in the merge sort). Last-minute note: in January 2024, the glibc developers have reverted this refactoring of qsort(), back to the original merge sort, plus a fallback to heap sort instead of quick sort; for more information: https://sourceware.org/pipermail/libc-alpha/2024-January/154051.html ======================================================================== Discussion ======================================================================== We have not tried to find a vulnerable program (i.e., a program that uses a nontransitive comparison function to qsort() attacker-controlled elements); however, vulnerable programs are certain to exist in the real world: - calls to qsort() are extremely common; - nontransitive comparison functions are also common; - all glibc versions from at least September 1992 (glibc 1.04, the first version that we could find online) to the current release (glibc 2.38) are affected by this memory corruption. Locally, forcing a malloc() failure in qsort() (which is necessary to reach the memory corruption) is easy: either execute the target program (e.g., a SUID-root program) with a low RLIMIT_AS, or allocate a large amount of memory with another program on the same machine. Remotely, forcing this malloc() failure is harder: either allocate a large amount of memory (e.g., a memory leak) in the network service that is being targeted, or in another network service on the same machine. ======================================================================== Acknowledgments ======================================================================== We thank the glibc security team and the linux-distros@...nwall. ======================================================================== Timeline ======================================================================== 2023-12-12: We sent a draft of our advisory to the glibc security team. They immediately acknowledged receipt of our email. 2023-12-19: The glibc security team decided to not treat this memory corruption in qsort() as a vulnerability in the glibc itself, as explained in the "Summary" of our advisory. 2024-01-16: We backported commit b9390ba to all current and past stable versions of the glibc, and sent this patch and a draft of our advisory to the linux-distros@...nwall (to piggyback on the glibc embargo for CVE-2023-6246). They immediately acknowledged receipt of our email. 2024-01-30: Coordinated Release Date (18:00 UTC).
Powered by blists - more mailing lists
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.