Bug #11883 describes a problem of an unbound alloca() use in fnmatch(), leading to crash when unexpectedly long string is passed to fnmatch() as either pattern or string. The issue seems to have been addressed in the following commit: http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=f15ce4d8 which makes fnmatch() use malloc() when arguments exceed certain length. That bug report does not explicitly mention integer overflow in malloc ((n + 1) * sizeof (wchar_t)), which got mentioned in reporter's blog post about the issue: http://scarybeastsecurity.blogspot.com/2011/02/i-got-accidental-code-execution-via.html That integer overflow was not addressed in the patch and with certain patterns, it's possible to trigger out-of-bounds read crash. The corner case is when n is 1073741823 / 0x3fffffff, (n + 1) * sizeof (wchar_t) is 0 on IA32, which usually causes malloc() to return non-NULL. string_end argument passed to internal_fnwmatch is computed as wstring + n, resulting in internal_fnwmatch being called with string > string_end. As some checks for end-of-string are done as n == string_end rather than n >= string_end, oob read may occur. I was able to trigger the crash on Fedora Rawhide glibc-2.13.90-3.i686 using the following modified version of the original reproducer: #include <err.h> #include <fnmatch.h> #include <locale.h> #include <stdlib.h> #include <string.h> #include <stdio.h> int main(int argc, const char* argv[]) { char *pat, *str; size_t pat_len = 1000000, str_len = 0x3fffffff; setlocale(LC_ALL, "en_US.UTF8"); if (argc > 1) str_len = atol(argv[1]); if (argc > 2) pat_len = atol(argv[2]); pat = malloc(pat_len + 1); str = malloc(str_len + 1); if (!pat || !str) errx(1, "malloc() failed."); memset(pat, '?', pat_len); pat[pat_len] = '\0'; memset(str, 'A', str_len); str[str_len] = '\0'; printf("running fnmatch(\"?\"x%zu, \"A\"x%zu, 0)...\n", pat_len, str_len); printf("return: %d\n", fnmatch(pat, str, 0)); return 0; }
I believe a check like this should be consistent with how other integer overflow checks are done in glibc: --- fnmatch.c.orig +++ fnmatch.c @@ -370,7 +370,8 @@ { prepare_wpattern: n = mbsrtowcs (NULL, &pattern, 0, &ps); - if (__builtin_expect (n == (size_t) -1, 0)) + if (__builtin_expect (n == (size_t) -1 + || n >= (size_t) -1 / sizeof(wchar_t) - 1, 0)) /* Something wrong. XXX Do we have to set `errno' to something which mbsrtows hasn't already done? */ @@ -414,7 +415,8 @@ { prepare_wstring: n = mbsrtowcs (NULL, &string, 0, &ps); - if (__builtin_expect (n == (size_t) -1, 0)) + if (__builtin_expect (n == (size_t) -1 + || n >= (size_t) -1 / sizeof(wchar_t) - 1, 0)) /* Something wrong. XXX Do we have to set `errno' to something which mbsrtows hasn't already done? */ Or something like this for readability: --- fnmatch.c.orig +++ fnmatch.c @@ -420,8 +420,11 @@ already done? */ goto free_return; - wstring_malloc = wstring - = (wchar_t *) malloc ((n + 1) * sizeof (wchar_t)); + if (__builtin_expect (n >= (size_t) -1 / sizeof(wchar_t) - 1, 0)) + wstring = NULL; + else + wstring_malloc = wstring + = (wchar_t *) malloc ((n + 1) * sizeof (wchar_t)); if (wstring == NULL) { free (wpattern_malloc);
Should be fixed correctly in git.