New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 639128 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Integer-overflow in media::mp2t::EsParserH264::UpdateVideoDecoderConfig

Project Member Reported by ClusterFuzz, Aug 18 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5483280269049856

Fuzzer: libfuzzer_es_parser_h264_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  media::mp2t::EsParserH264::UpdateVideoDecoderConfig
  media::mp2t::EsParserH264::EmitFrame
  media::mp2t::EsParserH264::ParseFromEsQueue
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_ubsan&range=398351:399229

Minimized Testcase (0.54 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94szq6XhuTYRuAgyG2R2m7tUA_W7kWjVDnHggpMIP6tRa0Nk_26japgI2SWrzSVMTWifv7xpPTDpPQrkH48vdKTyWQsxs6ut3CigGNYt9lLA7oA5-yADNyYPe2fu4AyouVyzS7GDIFmj3SZVAjUyKpIOOgmcg?testcase_id=5483280269049856

Issue manually filed by: mummareddy

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Labels: findit-wrong Te-Logged M-53
Owner: dalecur...@chromium.org
Status: Assigned (was: Untriaged)
From findit tool:

Author: dalecurtis
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/39a7f93d67f79d6afadb0f74254eef19b5ff9318
Time: Tue Jul 19 18:34:59 2016
The CL last changed line 18 of file es_parser_h264_fuzzer.cc, which is stack frame 3.
Owner: wolenetz@chromium.org
=>wolenetz for triage.
Project Member

Comment 3 by ClusterFuzz, Aug 25 2016

ClusterFuzz has detected this issue as fixed in range 413961:414068.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5483280269049856

Fuzzer: libfuzzer_es_parser_h264_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  media::mp2t::EsParserH264::UpdateVideoDecoderConfig
  media::mp2t::EsParserH264::EmitFrame
  media::mp2t::EsParserH264::ParseFromEsQueue
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_ubsan&range=398351:399229
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_ubsan&range=413961:414068

Minimized Testcase (0.54 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94szq6XhuTYRuAgyG2R2m7tUA_W7kWjVDnHggpMIP6tRa0Nk_26japgI2SWrzSVMTWifv7xpPTDpPQrkH48vdKTyWQsxs6ut3CigGNYt9lLA7oA5-yADNyYPe2fu4AyouVyzS7GDIFmj3SZVAjUyKpIOOgmcg?testcase_id=5483280269049856

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.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 4 by ClusterFuzz, Aug 25 2016

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
I don't see a related change that fixed this. I'll try a local repro to see what's up.
Cc: wolenetz@chromium.org
Owner: servolk@chromium.org
Status: Assigned (was: Verified)
From code inspection, over to servolk@, who looked at similar as part of fixing  bug 541669  (https://codereview.chromium.org/1896533002/).

The problem is that H264SPS int fields like "pic_*_in_*_minus1" are never verified to be less than std::numeric_limits<int>::max() before adding 1 to them. Please add further checks around this to prevent overflow.
Cc: w...@chromium.org sande...@chromium.org
CC+= watk@ and sandersd@ in case they want to take a stab at fixing this. Please coordinate, folks. The fix should be simple. I didn't actually confirm local repro, but looking at code, we read bits into an int, then add 1 without ever checking that the addition won't overflow. This is both es_parser_264.cc and likely also in h264_decoder.cc (see the CL referenced in c#6).
Owner: sande...@chromium.org
Status: Started (was: Assigned)
Taking over fix. I'm planning to eventually move all of these into H264SPS, but for now I'm just going to change all of the implementations that do it this way (there are three).
Labels: ClusterFuzz-Wrong
Per #4 and #6, marking ClusterFuzz-Wrong.
Status: Fixed (was: Started)
I guess ClusterFuzz isn't going to chime in on these changes anymore, but this should have been fixed some time ago.
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

Sign in to add a comment