New issue
Advanced search Search tips

Issue 635672 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Undefined-shift in ogm_packet

Project Member Reported by ClusterFuzz, Aug 8 2016

Issue description

Components: Blink
Components: -Blink Internals>Media>Codecs
Cc: jrumm...@chromium.org dalecur...@chromium.org
Status: Available (was: Untriaged)
can anybody take a look?
This is in the ffmpeg code, but looks like somebody that knows the ogg format will need to look at it.

https://cs.chromium.org/chromium/src/third_party/ffmpeg/libavformat/oggparseogm.c?l=174
174:    lb = ((*p & 2) << 1) | ((*p >> 6) & 3);
178:    while (lb--)
179:        os->pduration += p[lb+1] << (lb*8);

So lb can be 0-7, and it will shift bytes by 8/16/24/32/40/48. But pduration is a 32-bit int, so shifts of 32/40/48 don't make sense. Not sure what should happen to pduration when lb is large.

It appears that this field represents the duration represented as 0-7 bytes. So if the top few bytes are not 0, then the duration exceeds 32-bit int. So maybe the code needs to become:
    lb = ((*p & 2) << 1) | ((*p >> 6) & 3);
    while (lb-- >= 4)
        if (p[lb+1] != 0)
            return <some-sort-of-error>;
    while (lb--)
        os->pduration += p[lb+1] << (lb*8);

I checked the latest ffmpeg source (https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/oggparseogm.c) and this code is unchanged, so upgrading to the current ffmpeg won't fix this.
Cc: chcunningham@chromium.org wolenetz@chromium.org
Labels: Pri-2
Project Member

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

Comment 8 by ClusterFuzz, Oct 4 2017

Labels: Test-Predator-AutoOwner
Owner: thakis@chromium.org
Status: Assigned (was: Available)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/7d9f5804e4cc4bb6cc55133137a6e2060aa106b7 (roll clang 255169:257953).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Labels: Test-Predator-Wrong-CLs
Owner: ----
This bug predates that CL by months.
Owner: chcunningham@chromium.org
=>Chris for ffmpeg roll.
Labels: -Test-Predator-AutoOwner Test-Predator-Auto-Owner
Project Member

Comment 12 by ClusterFuzz, Nov 19 2017

ClusterFuzz has detected this issue as fixed in range 517698:517712.

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

Fuzzer: ochang_search_index_mutator
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  ogm_packet
  ogg_packet
  ogg_read_packet
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=370022:370027
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=517698:517712

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

See https://github.com/google/clusterfuzz-tools 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 13 by ClusterFuzz, Nov 19 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5341060371054592 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