Stack-overflow in policy::RegistryDict::~RegistryDict |
|||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5838889758752768 Fuzzer: libFuzzer_preg_parser_fuzzer Job Type: libfuzzer_chrome_msan Platform Id: linux Crash Type: Stack-overflow Crash Address: 0x7ffc35870ff0 Crash State: policy::RegistryDict::~RegistryDict Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=518239:518261 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5838889758752768 Issue filed automatically. See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
,
Jan 4 2018
Predator has provided 4 possible suspects but not sure about the culprit CL 1. Fix policy histogram PRESUBMIT by pmarko@chromium.org 2. Fix else-if after returns/breaks in components/. by thestig@chromium.org 3. New Device policy to disallow ARC on unaffiliated users by phweiss@google.com 4. Cull registered-but-unused crash keys from the registration lists. by rsesek@chromium.org cc'ing the above suspects owners for further triage. Thanks!
,
Jan 8 2018
I reproduced this and looked at it with gdb, and I don't think it is related to one listed CLs. The responsible fuzzer is here: https://cs.chromium.org/chromium/src/components/policy/core/common/preg_parser_fuzzer.cc?rcl=3659b506db01869ee2428ab5585646144309daea&l=36 It seems that the fuzzer provides data for a RegistryDict that is nested 36892 times (that is how often a breakpoint was hit during the construction of one new directory). For construction, this is not a problem, but as soon as the destructor of the root dict is called, we get that a multiple of that many nested function calls, resulting in the stack overflow. I'm assigning to ljusten, who wrote the fuzzer and some of the related code.
,
Jan 9 2018
Not related to my CL.
,
Jan 9 2018
Thanks for the analysis Philipp. Lutz, sounds like an iteration limit in HandleRecord's registry path handling (preg_parser.cc) would fix this?
,
Jan 9 2018
Thanks for taking a look. Seems like we're bailing during a std::map destruction, so there's not much we can do there and limiting the stack depth seems like the best option. In practice, the depth shouldn't be much greater than 10, so a limit of 1024 should be plenty. Still a bit surprised that std::map fails that early, but I guess it makes sense with the default stack size (1MB?).
,
Jan 9 2018
Note that there are probably multiple nested function calls involved for each RegistryDict nest level, as the structure looks somewhat like RegistryDict -> map(keys_) -> std::pair -> unique_ptr -> RegistryDict (+ whatever map uses internally).
,
Jan 9 2018
@early fail: The stacktrace on clusterfuzz.com is cut-off. On my machine, the stacktrace on the crash actually has a six-digit length. Here are the last 9 lines: #261746 destroy<std::__1::pair<std::__1::basic_string<char> const, std::__1::unique_ptr<policy::RegistryDict, std::__1::default_delete<policy::RegistryDict> > > > (__p=0x7040001c67b8, __a=...) at ../../buildtools/third_party/libc++/trunk/include/memory:1550 #261747 std::__1::__tree<std::__1::__value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::unique_ptr<policy::RegistryDict, std::__1::default_delete<policy::RegistryDict> > >, std::__1::__map_value_compare<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::unique_ptr<policy::RegistryDict, std::__1::default_delete<policy::RegistryDict> > >, policy::CaseInsensitiveStringCompare, true>, std::__1::allocator<std::__1::__value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::unique_ptr<policy::RegistryDict, std::__1::default_delete<policy::RegistryDict> > > > >::destroy (this=<optimized out>, __nd=0x704000000040) at ../../buildtools/third_party/libc++/trunk/include/__tree:1833 #261748 0x00000000004bb1a4 in std::__1::__tree<std::__1::__value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::unique_ptr<policy::RegistryDict, std::__1::default_delete<policy::RegistryDict> > >, std::__1::__map_value_compare<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::unique_ptr<policy::RegistryDict, std::__1::default_delete<policy::Regis---Type <return> to continue, or q <return> to quit--- tryDict> > >, policy::CaseInsensitiveStringCompare, true>, std::__1::allocator<std::__1::__value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::unique_ptr<policy::RegistryDict, std::__1::default_delete<policy::RegistryDict> > > > >::clear (this=<optimized out>) at ../../buildtools/third_party/libc++/trunk/include/__tree:1870 #261749 0x00000000004b5846 in policy::RegistryDict::~RegistryDict ( this=0x7fffffffbe60) at ../../components/policy/core/common/registry_dict.cc:158 #261750 0x00000000004933c9 in LLVMFuzzerTestOneInput ( data=0x7ffff7f52000 "PReg\001", size=151524) at ../../components/policy/core/common/preg_parser_fuzzer.cc:42 #261751 0x00000000004fc92c in fuzzer::Fuzzer::ExecuteCallback ( this=0x713000000000, Data=0x7ffff7f78000 "PReg\001", Size=151524) at ../../third_party/libFuzzer/src/FuzzerLoop.cpp:515 #261752 0x00000000004bea3f in fuzzer::RunOneTest (F=0x713000000000, InputFilePath=<optimized out>, MaxLen=<optimized out>) at ../../third_party/libFuzzer/src/FuzzerDriver.cpp:280 #261753 0x00000000004ccca1 in fuzzer::FuzzerDriver (argc=0x702076000001, argv=0x0, Callback=0x703000000600) at ../../third_party/libFuzzer/src/FuzzerDriver.cpp:703 #261754 0x00000000005239e1 in main (argc=6, argv=0x7fffffffd948) at ../../third_party/libFuzzer/src/FuzzerMain.cpp:20
,
Jan 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7fe21736cde8fc94a1f60c3ccb9c551c4f5e2992 commit 7fe21736cde8fc94a1f60c3ccb9c551c4f5e2992 Author: Pavol Marko <pmarko@chromium.org> Date: Thu Jan 18 09:22:38 2018 Limit nesting of RegistryDicts in preg_parser Limit number of path components in preg_parser and thus nesting of the resulting RegistryDict tree. Also switch to SplitStringPiece to use less memory. Bug: 798764 Test: clusterfuzz repro and components_unittests --gtest_filter=PRegParserTest* Change-Id: I2a47832240f1a61dc7877d8086bba0b3347d3055 Reviewed-on: https://chromium-review.googlesource.com/870033 Reviewed-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org> Commit-Queue: Pavol Marko <pmarko@chromium.org> Cr-Commit-Position: refs/heads/master@{#530096} [modify] https://crrev.com/7fe21736cde8fc94a1f60c3ccb9c551c4f5e2992/components/policy/core/common/preg_parser.cc
,
Jan 19 2018
ClusterFuzz has detected this issue as fixed in range 529955:530274. Detailed report: https://clusterfuzz.com/testcase?key=5838889758752768 Fuzzer: libFuzzer_preg_parser_fuzzer Job Type: libfuzzer_chrome_msan Platform Id: linux Crash Type: Stack-overflow Crash Address: 0x7ffc35870ff0 Crash State: policy::RegistryDict::~RegistryDict Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=518239:518261 Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=529955:530274 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5838889758752768 See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Jan 19 2018
ClusterFuzz testcase 5838889758752768 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ClusterFuzz
, Jan 3 2018Labels: Test-Predator-Auto-Components