Prevent out-of-bounds reads for short FEC packets.

Prevent out-of-bounds reads for short FEC packets.

BUG=webrtc:4771
R=stefan@webrtc.org

Committed: https://crrev.com/70d5c475ddef7ed9f848df02446d222729ed04ec
Cr-Commit-Position: refs/heads/master@{#9514}

[pbos-webrtc, 2015-06-29 13:00:09]

The CQ bit was checked by pbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-29 13:00:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1219703002/1

[pbos-webrtc, 2015-06-29 13:02:09]

The CQ bit was unchecked by pbos@webrtc.org

[pbos-webrtc, 2015-06-29 13:02:10]

The CQ bit was checked by pbos@webrtc.org

[commit-bot: I haz the power, 2015-06-29 13:02:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1219703002/1

[commit-bot: I haz the power, 2015-06-29 13:02:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-29 13:02:16]

No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[stefan-webrtc, 2015-06-29 13:19:50]

https://codereview.webrtc.org/1219703002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc (right):

https://codereview.webrtc.org/1219703002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc:92:
ForwardErrorCorrection::ReceivedPacket* received_packet =
Can you switch this to a scoped_ptr?

https://codereview.webrtc.org/1219703002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc:94: received_packet->pkt =
new ForwardErrorCorrection::Packet;
Doesn't this require deletion at line 109, 123 and 136?

[pbos-webrtc, 2015-06-29 13:29:21]

feedback

[pbos-webrtc, 2015-06-29 13:30:08]

https://codereview.webrtc.org/1219703002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc (right):

https://codereview.webrtc.org/1219703002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc:92:
ForwardErrorCorrection::ReceivedPacket* received_packet =
On 2015/06/29 13:19:50, stefan-webrtc (holmer) wrote:
> Can you switch this to a scoped_ptr?

Done.

https://codereview.webrtc.org/1219703002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc:94: received_packet->pkt =
new ForwardErrorCorrection::Packet;
On 2015/06/29 13:19:50, stefan-webrtc (holmer) wrote:
> Doesn't this require deletion at line 109, 123 and 136?

No, deleted in ~ReceivedPacket.

[stefan-webrtc, 2015-06-29 13:33:21]

lgtm, but please verify my comment.

https://codereview.webrtc.org/1219703002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc (right):

https://codereview.webrtc.org/1219703002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc:139: second_received_packet;
Is this guaranteed to be null if you call release() on it before being set?

[pbos-webrtc, 2015-06-29 13:34:04]

https://codereview.webrtc.org/1219703002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc (right):

https://codereview.webrtc.org/1219703002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc:139: second_received_packet;
On 2015/06/29 13:33:21, stefan-webrtc (holmer) wrote:
> Is this guaranteed to be null if you call release() on it before being set?

This is default-constructed to nullptr.

[pbos-webrtc, 2015-06-29 13:34:28]

The CQ bit was checked by pbos@webrtc.org

[commit-bot: I haz the power, 2015-06-29 13:34:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1219703002/20001

[commit-bot: I haz the power, 2015-06-29 14:22:07]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-06-29 14:22:17]

Patchset 2 (id:??) landed as
https://crrev.com/70d5c475ddef7ed9f848df02446d222729ed04ec
Cr-Commit-Position: refs/heads/master@{#9514}