@@ -171,7 +171,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
AVPacket *in;
uint8_t unit_type, new_idr, sps_seen, pps_seen;
- int32_t nal_size;
+ uint32_t nal_size;
const uint8_t *buf;
const uint8_t *buf_end;
uint8_t *out;
@@ -202,18 +202,23 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
out_size = 0;
do {
- ret= AVERROR(EINVAL);
- if (buf + s->length_size > buf_end)
- goto fail;
-
+ /* possible overread ok due to padding */
for (nal_size = 0, i = 0; i<s->length_size; i++)
nal_size = (nal_size << 8) | buf[i];
buf += s->length_size;
- unit_type = *buf & 0x1f;
- if (nal_size > buf_end - buf || nal_size < 0)
- goto fail;
+ /* This check requires the cast as the right side might
+ * otherwise be promoted to an unsigned value. */
+ if ((int64_t)nal_size > buf_end - buf) {
+ ret = AVERROR_INVALIDDATA;
+ goto fail;
+ }
+
+ if (!nal_size)
+ continue;
+
+ unit_type = *buf & 0x1f;
if (unit_type == H264_NAL_SPS)
sps_seen = new_idr = 1;
1. Left shifts of signed values are undefined as soon as the result is no longer representable in the target type. Therefore make nal_size an uint32_t and drop the check for whether it is < 0. 2. The two checks for overreads (whether the length field is contained in the packet and whether the actual unit is contained in the packet) can be combined into one because the packet is padded, i.e. a potential overread caused by reading the length field without checking whether said length field is actually part of the packet's buffer is allowed as one always stays within the padding. But one has to be aware of a pitfall: The comparison must be performed in (at least) int64_t as otherwise buf_end - buf might be promoted to uint32_t in which case an already occured overread would appear as a very large number. A comment explaining this has been added, too. 3. Units of size zero are now silently dropped; the earlier code would instead read the first byte of the next length field (or the first byte of padding) to infer the type of the current unit. 4. Futhermore, the earlier code returned the wrong error code. This has been fixed, too. Fixes #8290. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- The check for zero size units is new. libavcodec/h264_mp4toannexb_bsf.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)