New issue
Advanced search Search tips

Issue 666877 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Undefined-shift in webrtc::BufferLevelFilter::Update

Project Member Reported by ClusterFuzz, Nov 18 2016

Issue description

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

Fuzzer: libfuzzer_neteq_rtp_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  webrtc::BufferLevelFilter::Update
  webrtc::DecisionLogic::FilterBufferLevel
  webrtc::DecisionLogic::GetDecision
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_ubsan&range=423119:423133

Minimized Testcase (0.81 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97d-je5FaHuTpkTsrPHpNn5TxRT7TAr-tWZUcvdtOBQTtf55IrQ3XtKuWRS1i9u7kTTvB8q-YljNq9MmtenvKQrnNDS89iXK4DVl6bhqerM5EYZB6VO1aLm4MSbLv7H8N7PY2s9-O1ZLPt5VilW1AAVH12Phw?testcase_id=5641614816182272

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Components: Blink>WebRTC
Owner: kwiberg@chromium.org
Status: Assigned (was: Untriaged)
kwiberg @ could you please look into this.please feel free to re-assigned back if needed. thanks in advance !
might be a suspected Changelist: https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+/b9a5d2a304565d81be4a562deabd8397cc9a818a
Project Member

Comment 2 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
Cc: kwiberg@chromium.org
Owner: hlundin@chromium.org
I thought this would be a simple case of replacing << x with * (1 << x), but the left shift that fails is this one:

    filtered_current_level_ = std::max(0,
        filtered_current_level_ -
        (time_stretched_samples << 8) / static_cast<int>(packet_len_samples));

I'm not 100% sure, but it looks like time_stretched_samples may be intended to never be negative, which would make this bug much more interesting than the typical "left shift of negative value" bug.

Reassigning to Dr. NetEq.
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/8731176b922ad7d82d0407f2ac36d9f5396f9ac2

commit 8731176b922ad7d82d0407f2ac36d9f5396f9ac2
Author: Henrik Lundin <henrik.lundin@webrtc.org>
Date: Mon Oct 23 11:56:47 2017

NetEq: Fix an UBSan error

UBSan will trigger when shifting a negative value. This change avoids
that by replacing "x << 8" with "x * (1 << 8)".

Bug:  chromium:666877 
Change-Id: Ic89bd98e5a3feff35075df96b104b386cb4d8803
Reviewed-on: https://webrtc-review.googlesource.com/14552
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20387}
[modify] https://crrev.com/8731176b922ad7d82d0407f2ac36d9f5396f9ac2/modules/audio_coding/neteq/buffer_level_filter.cc

Project Member

Comment 6 by ClusterFuzz, Oct 24 2017

ClusterFuzz has detected this issue as fixed in range 510927:510949.

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

Fuzzer: libFuzzer_neteq_rtp_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  webrtc::BufferLevelFilter::Update
  webrtc::DecisionLogic::FilterBufferLevel
  webrtc::DecisionLogic::GetDecision
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=423119:423133
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=510927:510949

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

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.

Comment 7 by mmoroz@chromium.org, Oct 24 2017

For more information, please see https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md.

The link referenced in the description is no longer valid.

(bulk edit)
Status: Fixed (was: Started)
Labels: M-64
Project Member

Comment 10 by ClusterFuzz, Oct 26 2017

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