| Message ID | 20240502004150.3627661-7-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 | expand |
Michael Niedermayer: > Fixes: CID1439654 Untrusted pointer read > > Sponsored-by: Sovereign Tech Fund > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/cbs_jpeg.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c > index b1b58dcd65e..406147c082c 100644 > --- a/libavcodec/cbs_jpeg.c > +++ b/libavcodec/cbs_jpeg.c > @@ -146,13 +146,13 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx, > } > } else { > i = start; > - if (i + 2 > frag->data_size) { > + if (i > frag->data_size - 2) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: " > "truncated at %02x marker.\n", marker); > return AVERROR_INVALIDDATA; > } > length = AV_RB16(frag->data + i); > - if (i + length > frag->data_size) { > + if (length > frag->data_size - i) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: " > "truncated at %02x marker segment.\n", marker); > return AVERROR_INVALIDDATA; You should always mention when you are not fixing bugs in our code, but rather intend to apply workaround for coverity crazyness (i.e. the requirement that reading values in non-native endianness needs to be sanitized). - Andreas
On Thu, May 02, 2024 at 08:58:17AM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: CID1439654 Untrusted pointer read > > > > Sponsored-by: Sovereign Tech Fund > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/cbs_jpeg.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c > > index b1b58dcd65e..406147c082c 100644 > > --- a/libavcodec/cbs_jpeg.c > > +++ b/libavcodec/cbs_jpeg.c > > @@ -146,13 +146,13 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx, > > } > > } else { > > i = start; > > - if (i + 2 > frag->data_size) { > > + if (i > frag->data_size - 2) { > > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: " > > "truncated at %02x marker.\n", marker); > > return AVERROR_INVALIDDATA; > > } > > length = AV_RB16(frag->data + i); > > - if (i + length > frag->data_size) { > > + if (length > frag->data_size - i) { > > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: " > > "truncated at %02x marker segment.\n", marker); > > return AVERROR_INVALIDDATA; > > You should always mention when you are not fixing bugs in our code, but > rather intend to apply workaround for coverity crazyness (i.e. the > requirement that reading values in non-native endianness needs to be > sanitized). writing code like if (i + length > frag->data_size) is IMHO not proper and that has nothing to do with coverity the reason why this is not proper is that i + length could in principle overflow. It depends on teh whole loop and calling code if that is possible or not. to check length, length should be alone on one side of the check thx [...]
diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c index b1b58dcd65e..406147c082c 100644 --- a/libavcodec/cbs_jpeg.c +++ b/libavcodec/cbs_jpeg.c @@ -146,13 +146,13 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx, } } else { i = start; - if (i + 2 > frag->data_size) { + if (i > frag->data_size - 2) { av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: " "truncated at %02x marker.\n", marker); return AVERROR_INVALIDDATA; } length = AV_RB16(frag->data + i); - if (i + length > frag->data_size) { + if (length > frag->data_size - i) { av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: " "truncated at %02x marker segment.\n", marker); return AVERROR_INVALIDDATA;
Fixes: CID1439654 Untrusted pointer read Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/cbs_jpeg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)