New issue
Advanced search Search tips

Issue 834629 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Integer-overflow in blink::ParseDate

Project Member Reported by ClusterFuzz, Apr 19 2018

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5902106654146560

Fuzzer: libFuzzer_mhtml_parser_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::ParseDate
  blink::MIMEHeader::ParseHeader
  blink::MHTMLParser::ParseArchiveWithHeader
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=547824:547836

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5902106654146560

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
 
Project Member

Comment 1 by ClusterFuzz, Apr 19 2018

Components: Platform
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Apr 19 2018

Labels: Test-Predator-Auto-Owner
Owner: dewittj@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/cbd393505608eae84929c78ea1249ee2cf8e9121 ([Offline Pages] Extract main-frame URL from mhtml when rendered.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee1a7ef4d3573b44b751f51b122d2fa1eccc3caf

commit ee1a7ef4d3573b44b751f51b122d2fa1eccc3caf
Author: Justin DeWitt <dewittj@chromium.org>
Date: Fri Apr 20 17:18:30 2018

[MHTML] Fix crashes in date header parsing.

Previously the MHTML parser would use ParseDate which has bugs for several
values of the date string.  This issue is fixed by using
WTF::Time::FromString instead.  Fuzzer group link:
https://clusterfuzz.com/v2/testcases?q=group%3A6154265509494784

Note that this does not fix the underlying issue in ParseDate.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Bug:  831864 , 834629 
Change-Id: Ib0f8210e42667bb031dd13b0c6c86204bab08822
Reviewed-on: https://chromium-review.googlesource.com/1020362
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552365}
[modify] https://crrev.com/ee1a7ef4d3573b44b751f51b122d2fa1eccc3caf/third_party/blink/renderer/platform/mhtml/mhtml_parser.cc
[modify] https://crrev.com/ee1a7ef4d3573b44b751f51b122d2fa1eccc3caf/third_party/blink/renderer/platform/mhtml/mhtml_parser_test.cc

Project Member

Comment 4 by ClusterFuzz, Apr 21 2018

ClusterFuzz has detected this issue as fixed in range 552355:552365.

Detailed report: https://clusterfuzz.com/testcase?key=5902106654146560

Fuzzer: libFuzzer_mhtml_parser_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::ParseDate
  blink::MIMEHeader::ParseHeader
  blink::MHTMLParser::ParseArchiveWithHeader
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=547824:547836
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=552355:552365

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5902106654146560

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.
Project Member

Comment 5 by ClusterFuzz, Apr 21 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5902106654146560 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Merge-Request-67
Project Member

Comment 7 by sheriffbot@chromium.org, Apr 25 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by gov...@chromium.org, Apr 25 2018

Pls merge your change to M67 branch 3396 ASAP so we can pick it up for next M67 Beta release. Thank you.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 25 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e1ab01f21e2493c51060fb7d623e6e001f3cae3

commit 2e1ab01f21e2493c51060fb7d623e6e001f3cae3
Author: Justin DeWitt <dewittj@chromium.org>
Date: Wed Apr 25 16:09:53 2018

[MHTML] Fix crashes in date header parsing.

Previously the MHTML parser would use ParseDate which has bugs for several
values of the date string.  This issue is fixed by using
WTF::Time::FromString instead.  Fuzzer group link:
https://clusterfuzz.com/v2/testcases?q=group%3A6154265509494784

Note that this does not fix the underlying issue in ParseDate.

TBR=dewittj@chromium.org

(cherry picked from commit ee1a7ef4d3573b44b751f51b122d2fa1eccc3caf)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Bug:  831864 , 834629 
Change-Id: Ib0f8210e42667bb031dd13b0c6c86204bab08822
Reviewed-on: https://chromium-review.googlesource.com/1020362
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552365}
Reviewed-on: https://chromium-review.googlesource.com/1028355
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#292}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/2e1ab01f21e2493c51060fb7d623e6e001f3cae3/third_party/blink/renderer/platform/mhtml/mhtml_parser.cc
[modify] https://crrev.com/2e1ab01f21e2493c51060fb7d623e6e001f3cae3/third_party/blink/renderer/platform/mhtml/mhtml_parser_test.cc

Sign in to add a comment