| Message ID | AS8P250MB0744914D7591F027FA706A7E8F522@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/bsf/(hevc|vvc)_mp4toannexb: Ensure extradata_size < INT_MAX | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/commit_msg_loongarch64 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 2/17/2024 11:41 PM, Andreas Rheinhardt wrote: > AVCodecParameters.extradata_size is an int. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavcodec/bsf/hevc_mp4toannexb.c | 2 +- > libavcodec/bsf/vvc_mp4toannexb.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/bsf/hevc_mp4toannexb.c b/libavcodec/bsf/hevc_mp4toannexb.c > index 8eec18f31e..c0df2b79a6 100644 > --- a/libavcodec/bsf/hevc_mp4toannexb.c > +++ b/libavcodec/bsf/hevc_mp4toannexb.c > @@ -69,7 +69,7 @@ static int hevc_extradata_to_annexb(AVBSFContext *ctx) > > if (!nalu_len || > nalu_len > bytestream2_get_bytes_left(&gb) || > - 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX - new_extradata_size) { > + 4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) - AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) { > ret = AVERROR_INVALIDDATA; > goto fail; > } > diff --git a/libavcodec/bsf/vvc_mp4toannexb.c b/libavcodec/bsf/vvc_mp4toannexb.c > index 36bdae8f49..1b851f3223 100644 > --- a/libavcodec/bsf/vvc_mp4toannexb.c > +++ b/libavcodec/bsf/vvc_mp4toannexb.c > @@ -159,7 +159,7 @@ static int vvc_extradata_to_annexb(AVBSFContext *ctx) > > if (!nalu_len || > nalu_len > bytestream2_get_bytes_left(&gb) || > - 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX - new_extradata_size) { > + 4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) - AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) { Just use INT_MAX, there's no point in this check. Do you expect a system where an int is smaller than the type meant to store size of buffers in memory? > ret = AVERROR_INVALIDDATA; > goto fail; > }
James Almer: > On 2/17/2024 11:41 PM, Andreas Rheinhardt wrote: >> AVCodecParameters.extradata_size is an int. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavcodec/bsf/hevc_mp4toannexb.c | 2 +- >> libavcodec/bsf/vvc_mp4toannexb.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavcodec/bsf/hevc_mp4toannexb.c >> b/libavcodec/bsf/hevc_mp4toannexb.c >> index 8eec18f31e..c0df2b79a6 100644 >> --- a/libavcodec/bsf/hevc_mp4toannexb.c >> +++ b/libavcodec/bsf/hevc_mp4toannexb.c >> @@ -69,7 +69,7 @@ static int hevc_extradata_to_annexb(AVBSFContext *ctx) >> if (!nalu_len || >> nalu_len > bytestream2_get_bytes_left(&gb) || >> - 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > >> SIZE_MAX - new_extradata_size) { >> + 4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) - >> AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) { >> ret = AVERROR_INVALIDDATA; >> goto fail; >> } >> diff --git a/libavcodec/bsf/vvc_mp4toannexb.c >> b/libavcodec/bsf/vvc_mp4toannexb.c >> index 36bdae8f49..1b851f3223 100644 >> --- a/libavcodec/bsf/vvc_mp4toannexb.c >> +++ b/libavcodec/bsf/vvc_mp4toannexb.c >> @@ -159,7 +159,7 @@ static int vvc_extradata_to_annexb(AVBSFContext *ctx) >> if (!nalu_len || >> nalu_len > bytestream2_get_bytes_left(&gb) || >> - 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > >> SIZE_MAX - new_extradata_size) { >> + 4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) - >> AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) { > > Just use INT_MAX, there's no point in this check. Do you expect a system > where an int is smaller than the type meant to store size of buffers in > memory? > We typically try to avoid such assumptions (although we actually have lots of INT_MAX <= SIZE_MAX assumptions in the codebase, namely lots of instances where one uses an int and an allocation function). In this case using FFMIN(INT_MAX, SIZE_MAX) will make it easier to find these lines via git grep when extradata_size is switched to size_t. Alternatively, one could add an AV_CODECPAR_MAX_EXTRADATA_SIZE define, but this is even longer than FFMIN(INT_MAX, SIZE_MAX). - Andreas
diff --git a/libavcodec/bsf/hevc_mp4toannexb.c b/libavcodec/bsf/hevc_mp4toannexb.c index 8eec18f31e..c0df2b79a6 100644 --- a/libavcodec/bsf/hevc_mp4toannexb.c +++ b/libavcodec/bsf/hevc_mp4toannexb.c @@ -69,7 +69,7 @@ static int hevc_extradata_to_annexb(AVBSFContext *ctx) if (!nalu_len || nalu_len > bytestream2_get_bytes_left(&gb) || - 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX - new_extradata_size) { + 4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) - AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) { ret = AVERROR_INVALIDDATA; goto fail; } diff --git a/libavcodec/bsf/vvc_mp4toannexb.c b/libavcodec/bsf/vvc_mp4toannexb.c index 36bdae8f49..1b851f3223 100644 --- a/libavcodec/bsf/vvc_mp4toannexb.c +++ b/libavcodec/bsf/vvc_mp4toannexb.c @@ -159,7 +159,7 @@ static int vvc_extradata_to_annexb(AVBSFContext *ctx) if (!nalu_len || nalu_len > bytestream2_get_bytes_left(&gb) || - 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX - new_extradata_size) { + 4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) - AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) { ret = AVERROR_INVALIDDATA; goto fail; }
AVCodecParameters.extradata_size is an int. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/bsf/hevc_mp4toannexb.c | 2 +- libavcodec/bsf/vvc_mp4toannexb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)