Issue metadata
Sign in to add a comment
|
Security: libicu has buffer overflow in path traversal code |
||||||||||||||||||||||
Issue descriptionVULNERABILITY DETAILS The code in libicu looking for a file in /usr/share/zoneinfo matching given timezone data has a buffer overflow. It assumes that all paths are shorter than PATH_MAX (= 4096), which is not true at all. Here's the relevant code: https://cs.chromium.org/chromium/src/third_party/icu/source/common/putil.cpp?sq=package:chromium&dr=C&rcl=1475853592&l=940 Relevant snippet: char curpath[MAX_PATH_SIZE]; DIR* dirp = opendir(path); DIR* subDirp = NULL; struct dirent* dirEntry = NULL; [...snip...] /* Save the current path */ uprv_memset(curpath, 0, MAX_PATH_SIZE); uprv_strcpy(curpath, path); /* Check each entry in the directory. */ while((dirEntry = readdir(dirp)) != NULL) { const char* dirName = dirEntry->d_name; if (uprv_strcmp(dirName, SKIP1) != 0 && uprv_strcmp(dirName, SKIP2) != 0) { /* Create a newpath with the new entry to test each entry in the directory. */ char newpath[MAX_PATH_SIZE]; uprv_strcpy(newpath, curpath); uprv_strcat(newpath, dirName); [...snip...] } Clearly, if strlen(curpath) + strlen(dirname) > PATH_MAX, then uprv_strcat writes after newpath buffer. The impact probably isn't very large, as you need elevated privileges to add stuff to /usr/share/zoneinfo anyway. VERSION Chrome Version: all Operating System: Linux, probably others. REPRODUCTION CASE First you need to create very long path in /usr/share/zoneinfo. You'll likely need to temporarily move your regular timezone file away from /usr/share/zoneinfo so that it's not found before your crafted path. Then it works something like this: xyzzyz@xyzzyz:~/chromium/src$ gdb -- ./out/Default/base_unittests GNU gdb (GDB) 7.9-gg19 Copyright (C) 2015 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-grtev4-linux-gnu". Type "show configuration" for configuration details. <http://go/gdb-home FAQ: http://go/gdb-faq Email: c-toolchain-team IRC: gdb> Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word". ---- Loading Rust pretty-printers ---- Reading symbols from ./out/Default/base_unittests...done. Unable to determine compiler version. Skipping loading of libstdc++ pretty-printers for now. Non-google3 binary detected. (gdb) break ../../third_party/icu/source/common/putil.cpp:939 if strlen(curpath) + strlen(dirName) > 4096 Breakpoint 1 at 0x11224f9: file ../../third_party/icu/source/common/putil.cpp, line 939. (gdb) r Starting program: /usr/local/google/home/xyzzyz/chromium/src/out/Default/base_unittests [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/grte/v4/lib64/libthread_db.so.1". Debugger detected, switching to single process mode. Pass --test-launcher-debug-launcher to debug the launcher itself. Breakpoint 1, searchForTZFile ( path=0x7ffffffd9ff0 "/usr/share/zoneinfo/America/A", 'a' <repeats 171 times>..., tzInfo=0xc328ecb24a0) at ../../third_party/icu/source/common/putil.cpp:939 939 uprv_strcpy(newpath, curpath); (gdb) p strlen(curpath) $1 = 4060 (gdb) p strlen(dirName) $2 = 251 (gdb) n 940 uprv_strcat(newpath, dirName); (gdb) n 942 if ((subDirp = opendir(newpath)) != NULL) { (gdb) p strlen(newpath) $3 = 4311
,
Oct 10 2016
Should I create a bug in a public ICU tracker then?
,
Oct 10 2016
Same issue is present in isFileModTimeLater() https://cs.chromium.org/chromium/src/third_party/icu/source/tools/toolutil/filetools.cpp?q=isFileModTimeLater&sq=package:chromium&dr=CSs&l=62 though it also seems to haven no real impact, as it's only called from some packaging code.
,
Oct 10 2016
OS=Chrome, too even though it's even more difficult to change /usr/share/zoneinfo on Google Chrome OS. Just filed a bug against the upstream at http://bugs.icu-project.org/trac/ticket/12798 (not viewable because it's flagged as sensitive).
,
Oct 21 2016
Upstream fixed this issue a couple of days ago. I'll cherry-pick the change for M-55.
,
Nov 8 2016
This issue was fixed in ICU 58.1 and Chrome ToT has it. Let me port a fix back to ICU 56.1 in M55 branch.
,
Nov 11 2016
Requesting approval for merge to M55. https://codereview.chromium.org/2496433004/ The M56/trunk has ICU 58.1 with the above patch. M55 has ICU 56.1 that needs cherry-picking a patch from ICU 58.1.
,
Nov 12 2016
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 12 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Nov 13 2016
,
Nov 14 2016
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/578dcaea09e2d010a7644668b2e2f8d3eec3435f commit 578dcaea09e2d010a7644668b2e2f8d3eec3435f Author: Jungshik Shin <jungshik@google.com> Date: Mon Nov 14 22:06:04 2016
,
Nov 15 2016
Seems like this is already merged to M55 at #11. If there is nothing is pending for M55, please remove "Merge-Approved-55" label and apply "merge-merged-2883" label. Thank you.
,
Nov 15 2016
Label updated per comment 12
,
Nov 21 2016
,
Nov 29 2016
,
Feb 19 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by tsepez@chromium.org
, Oct 10 2016Owner: js...@chromium.org
Status: ExternalDependency (was: Unconfirmed)