Summary: | fnmatch: integer overflow in computation of the required memory (CVE-2011-1659) | ||
---|---|---|---|
Product: | glibc | Reporter: | Tomas Hoger <thoger> |
Component: | libc | Assignee: | Ulrich Drepper <drepper.fsp> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | fweimer |
Priority: | P2 | Flags: | fweimer:
security+
|
Version: | 2.13 | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: |
Description
Tomas Hoger
2011-03-14 14:43:18 UTC
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. |