Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1721593004:
iSAC float: Check for end of input buffer while decoding (Closed)
|
Created:
4 years, 10 months ago by kwiberg-webrtc Modified:
4 years, 10 months ago CC:
kwiberg-webrtc, tlegrand-webrtc, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, peah-webrtc, minyue-webrtc Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptioniSAC float: Check for end of input buffer while decoding
Previously, we relied on the encoded stream to come to an end before
the end of the buffer. This is a bad idea, since it is possible to
craft a stream that fills the buffer while decoding to less than the
expected amount of data; without the new checks introduced here, this
causes the decoder to read past the end of the input buffer.
BUG=chromium:582471, chromium:587852
Committed: https://crrev.com/b7261fd3ae7c31269b8c282e4c4c55dd3946db45
Cr-Commit-Position: refs/heads/master@{#11734}
Patch Set 1 : #
Total comments: 3
Patch Set 2 : comment #Messages
Total messages: 24 (12 generated)
Description was changed from ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to be long enough. This is a bad idea, since it is possible to craft a stream that decodes to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input stream. BUG=chromium:582471, chromium:587852 ========== to ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to be long enough. This is a bad idea, since it is possible to craft a stream that decodes to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input stream. BUG=chromium:582471, chromium:587852 ==========
kwiberg@webrtc.org changed reviewers: + henrik.lundin@webrtc.org, tina.legrand@webrtc.org
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to be long enough. This is a bad idea, since it is possible to craft a stream that decodes to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input stream. BUG=chromium:582471, chromium:587852 ========== to ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to come to an end before the end of the buffer. This is a bad idea, since it is possible to craft a stream that fills the buffer while decoding to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input buffer. BUG=chromium:582471, chromium:587852 ==========
LGTM with comments about comments. https://codereview.webrtc.org/1721593004/diff/40001/webrtc/modules/audio_codi... File webrtc/modules/audio_coding/codecs/isac/main/source/arith_routines_logist.c (right): https://codereview.webrtc.org/1721593004/diff/40001/webrtc/modules/audio_codi... webrtc/modules/audio_coding/codecs/isac/main/source/arith_routines_logist.c:193: if (stream_ptr + 3 - streamdata->stream >= STREAM_SIZE_MAX_60) Please, comment on why you use STREAM_SIZE_MAX_60 instead of STREAM_SIZE_MAX. https://codereview.webrtc.org/1721593004/diff/40001/webrtc/modules/audio_codi... webrtc/modules/audio_coding/codecs/isac/main/source/arith_routines_logist.c:282: if (stream_ptr + 1 - streamdata->stream >= STREAM_SIZE_MAX_60) and here.
https://codereview.webrtc.org/1721593004/diff/40001/webrtc/modules/audio_codi... File webrtc/modules/audio_coding/codecs/isac/main/source/arith_routines_logist.c (right): https://codereview.webrtc.org/1721593004/diff/40001/webrtc/modules/audio_codi... webrtc/modules/audio_coding/codecs/isac/main/source/arith_routines_logist.c:193: if (stream_ptr + 3 - streamdata->stream >= STREAM_SIZE_MAX_60) On 2016/02/23 13:16:10, hlundin-webrtc wrote: > Please, comment on why you use STREAM_SIZE_MAX_60 instead of STREAM_SIZE_MAX. I extracted a named constant. Pretty?
lgtm
The CQ bit was checked by kwiberg@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1721593004/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1721593004/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL) mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by kwiberg@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1721593004/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1721593004/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
lgtm
The CQ bit was checked by kwiberg@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1721593004/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1721593004/60001
Message was sent while issue was closed.
Description was changed from ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to come to an end before the end of the buffer. This is a bad idea, since it is possible to craft a stream that fills the buffer while decoding to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input buffer. BUG=chromium:582471, chromium:587852 ========== to ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to come to an end before the end of the buffer. This is a bad idea, since it is possible to craft a stream that fills the buffer while decoding to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input buffer. BUG=chromium:582471, chromium:587852 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to come to an end before the end of the buffer. This is a bad idea, since it is possible to craft a stream that fills the buffer while decoding to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input buffer. BUG=chromium:582471, chromium:587852 ========== to ========== iSAC float: Check for end of input buffer while decoding Previously, we relied on the encoded stream to come to an end before the end of the buffer. This is a bad idea, since it is possible to craft a stream that fills the buffer while decoding to less than the expected amount of data; without the new checks introduced here, this causes the decoder to read past the end of the input buffer. BUG=chromium:582471, chromium:587852 Committed: https://crrev.com/b7261fd3ae7c31269b8c282e4c4c55dd3946db45 Cr-Commit-Position: refs/heads/master@{#11734} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b7261fd3ae7c31269b8c282e4c4c55dd3946db45 Cr-Commit-Position: refs/heads/master@{#11734} |
