| Message ID | 20240626184440.1318-1-kasper93@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/jpegxl_parser: check remaining data buffer size | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Wed, 26 Jun 2024 at 21:00, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Kacper Michajłow: > > Fixes use of uninitialized value, reported by MSAN. > > > > Found by OSS-Fuzz. > > > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > > --- > > libavcodec/jpegxl_parser.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c > > index 8c45e1a1b7..8371d78a45 100644 > > --- a/libavcodec/jpegxl_parser.c > > +++ b/libavcodec/jpegxl_parser.c > > @@ -504,9 +504,14 @@ static int read_dist_clustering(GetBitContext *gb, JXLEntropyDecoder *dec, JXLDi > > return 0; > > } > > > > + if (get_bits_left(gb) <= 0) > > + return AVERROR_BUFFER_TOO_SMALL; > > + > > if (get_bits1(gb)) { > > /* simple clustering */ > > - uint32_t nbits = get_bits(gb, 2); > > + int nbits = get_bits(gb, 2); > > + if (get_bits_left(gb) < nbits * bundle->num_dist) > > + return AVERROR_BUFFER_TOO_SMALL; > > for (int i = 0; i < bundle->num_dist; i++) > > bundle->cluster_map[i] = get_bitsz(gb, nbits); > > } else { > > Where is the uninitialized value that you are speaking of? When the > implicit checks of the GetBit-API are enabled, the values when > overreading come from reading the padding which is supposed to be > initialized. Is it here? Indeed, it makes sense. If the padding is supposed to be initialized, I've sent small patches with memsets in the required places to fix all my failing test cases. Let me know if it makes sense. - Kacper
diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c index 8c45e1a1b7..8371d78a45 100644 --- a/libavcodec/jpegxl_parser.c +++ b/libavcodec/jpegxl_parser.c @@ -504,9 +504,14 @@ static int read_dist_clustering(GetBitContext *gb, JXLEntropyDecoder *dec, JXLDi return 0; } + if (get_bits_left(gb) <= 0) + return AVERROR_BUFFER_TOO_SMALL; + if (get_bits1(gb)) { /* simple clustering */ - uint32_t nbits = get_bits(gb, 2); + int nbits = get_bits(gb, 2); + if (get_bits_left(gb) < nbits * bundle->num_dist) + return AVERROR_BUFFER_TOO_SMALL; for (int i = 0; i < bundle->num_dist; i++) bundle->cluster_map[i] = get_bitsz(gb, nbits); } else {
Fixes use of uninitialized value, reported by MSAN. Found by OSS-Fuzz. Signed-off-by: Kacper Michajłow <kasper93@gmail.com> --- libavcodec/jpegxl_parser.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)