Out-of-memory in csv_reader_fuzzer |
|||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5163314629574656 Fuzzer: libFuzzer_csv_reader_fuzzer Fuzz target binary: csv_reader_fuzzer Job Type: libfuzzer_chrome_asan Platform Id: linux Crash Type: Out-of-memory (exceeds 2048 MB) Crash Address: Crash State: csv_reader_fuzzer Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan&range=604452:604462 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5163314629574656 Issue filed automatically. See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
,
Dec 7
Automatically adding ccs based on OWNERS file / target commit history. If this is incorrect, please add ClusterFuzz-Wrong label.
,
Dec 7
I have not looked in where the memory is spent in the testcase, but will try to propose reasonable per-string etc. limits once I do. It would be better if there were no overall cap on the number of password entries, but in the worst case, we might add it to avoid DDoS-ing. The good point is that this is only used for importing passwords, which is only behind a flag (and broken) and likely to stay this way. So there is no urgency fixing this. I'm adding battre@, the lead of the password manager team for information, in case the team restarted the import project soon. Also adding jdoerrie@ who I expect to bother with code reviews.
,
Dec 7
Fix at https://crrev.com/c/1368106
,
Dec 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f74ca2830bd37c48481207c67dbdd8952e38f54a commit f74ca2830bd37c48481207c67dbdd8952e38f54a Author: Vaclav Brozek <vabr@chromium.org> Date: Sat Dec 08 07:58:41 2018 Use StringPiece to save memory for CSV parsing results The CSV parsing routine from components/password_manager/core/browser/import/csv_reader.h stores the parsed data in a vector of string values from the first row, and a vector of maps with keys from that first row. So far, the keys have been std::string, which is rather inefficient, because it stores the same value N times, for N being the # of rows in the CSV file, while it could have been just stored once. Therefore, this CL changes the map keys to base::StringPiece. This also fixes the out-of-memory issue reported by the fuzzer in the associated bug (this was 100% reproducible before the CL). To make this change safe, the CL also encapsulates the vectors with parsed data in a class, so that the StringPiece does not end up dangling if the values from the first row have been destroyed. The CL does not rename the csv_reader.* files to match the new class name, to reduce churn. This can be done in a separate CL if required. Bug: 912783 Change-Id: I44bfbbdfe8125a88f80d0d655829519c2f0d4121 Reviewed-on: https://chromium-review.googlesource.com/c/1368106 Commit-Queue: Vaclav Brozek <vabr@chromium.org> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org> Cr-Commit-Position: refs/heads/master@{#614960} [modify] https://crrev.com/f74ca2830bd37c48481207c67dbdd8952e38f54a/components/password_manager/core/browser/export/password_csv_writer_unittest.cc [modify] https://crrev.com/f74ca2830bd37c48481207c67dbdd8952e38f54a/components/password_manager/core/browser/import/csv_reader.cc [modify] https://crrev.com/f74ca2830bd37c48481207c67dbdd8952e38f54a/components/password_manager/core/browser/import/csv_reader.h [modify] https://crrev.com/f74ca2830bd37c48481207c67dbdd8952e38f54a/components/password_manager/core/browser/import/csv_reader_unittest.cc [modify] https://crrev.com/f74ca2830bd37c48481207c67dbdd8952e38f54a/components/password_manager/core/browser/import/password_csv_reader.cc [modify] https://crrev.com/f74ca2830bd37c48481207c67dbdd8952e38f54a/components/password_manager/core/browser/import/password_csv_reader.h
,
Dec 8
,
Dec 8
ClusterFuzz has detected this issue as fixed in range 614959:614960. Detailed report: https://clusterfuzz.com/testcase?key=5163314629574656 Fuzzer: libFuzzer_csv_reader_fuzzer Fuzz target binary: csv_reader_fuzzer Job Type: libfuzzer_chrome_asan Platform Id: linux Crash Type: Out-of-memory (exceeds 2048 MB) Crash Address: Crash State: csv_reader_fuzzer Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan&range=604452:604462 Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan&range=614959:614960 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5163314629574656 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.
,
Dec 8
ClusterFuzz testcase 5163314629574656 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
, Dec 7