Prevent OOB read on truncated H264 headers.

Prevent OOB read on truncated H264 headers.

Prevents OOB reads on truncated FU-A NAL units, StapA headers and past
truncation just after StapA headers.

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

Committed: https://crrev.com/2f1509395b56fe3175b27dc2ac76e8f749c809f7
Cr-Commit-Position: refs/heads/master@{#9522}

[pbos-webrtc, 2015-06-30 12:19:34]

PTAL

[pbos-webrtc, 2015-06-30 12:19:42]

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

[commit-bot: I haz the power, 2015-06-30 12:19:54]

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

[pbos-webrtc, 2015-06-30 12:24:31]

rename test

[pbos-webrtc, 2015-06-30 12:35:17]

use kFuAHeaderSize constant

[stefan-webrtc, 2015-06-30 13:08:06]

Feel free to do more than one fix per cl if they are in the same file...

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc (right):

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:55: uint8_t nal_type =
payload_data[0] & kTypeMask;
Seems like you would want to fix this as well

[pbos-webrtc, 2015-06-30 13:11:34]

Agreed, I generally want to find them by fuzzing though, so I try to fix only
one bug at a time so I also get regression input for them.

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc (right):

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:55: uint8_t nal_type =
payload_data[0] & kTypeMask;
On 2015/06/30 13:08:06, stefan-webrtc (holmer) wrote:
> Seems like you would want to fix this as well

Covered by size != 0 check in ::Parse.

[stefan-webrtc, 2015-06-30 13:12:43]

On 2015/06/30 13:11:34, pbos-webrtc wrote:
> Agreed, I generally want to find them by fuzzing though, so I try to fix only
> one bug at a time so I also get regression input for them.
> 
>
https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
> File webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc (right):
> 
>
https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
> webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:55: uint8_t nal_type =
> payload_data[0] & kTypeMask;
> On 2015/06/30 13:08:06, stefan-webrtc (holmer) wrote:
> > Seems like you would want to fix this as well
> 
> Covered by size != 0 check in ::Parse.

Right, but you can still do that but not upload until you think the file passes.
But maybe that's difficult since you test such a big class (RTP). :/

[stefan-webrtc, 2015-06-30 13:13:19]

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc (right):

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:58: nal_type =
payload_data[kStapAHeaderSize] & kTypeMask;
Is this also safe?

[pbos-webrtc, 2015-06-30 13:13:45]

On 2015/06/30 13:12:43, stefan-webrtc (holmer) wrote:
> On 2015/06/30 13:11:34, pbos-webrtc wrote:
> > Agreed, I generally want to find them by fuzzing though, so I try to fix
only
> > one bug at a time so I also get regression input for them.
> > 
> >
>
https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
> > File webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc (right):
> > 
> >
>
https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
> > webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:55: uint8_t nal_type =
> > payload_data[0] & kTypeMask;
> > On 2015/06/30 13:08:06, stefan-webrtc (holmer) wrote:
> > > Seems like you would want to fix this as well
> > 
> > Covered by size != 0 check in ::Parse.
> 
> Right, but you can still do that but not upload until you think the file
passes.
> But maybe that's difficult since you test such a big class (RTP). :/

Yeah, I've aggregated some H264 errors for after this one. :)

[pbos-webrtc, 2015-06-30 13:16:20]

additional StapANalu overflows

[pbos-webrtc, 2015-06-30 13:16:37]

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc (right):

https://codereview.webrtc.org/1218023003/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:58: nal_type =
payload_data[kStapAHeaderSize] & kTypeMask;
On 2015/06/30 13:13:18, stefan-webrtc (holmer) wrote:
> Is this also safe?

No, merging in changes from another CL which fit here.

[pbos-webrtc, 2015-06-30 13:18:28]

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

[commit-bot: I haz the power, 2015-06-30 13:18:34]

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

[stefan-webrtc, 2015-06-30 13:19:10]

lgtm

[pbos-webrtc, 2015-06-30 13:20:00]

The CQ bit was unchecked by pbos@webrtc.org

[pbos-webrtc, 2015-06-30 13:20:00]

The CQ bit was checked by pbos@webrtc.org

[commit-bot: I haz the power, 2015-06-30 13:20:14]

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

[commit-bot: I haz the power, 2015-06-30 15:23:40]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-06-30 15:23:50]

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