| Message ID | 20230802000135.26482-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/jpeg2000htdec: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k() | 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 Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > > This also fixes assertion failures > > Fixes: shift exponent 95 is too large for 64-bit type 'unsigned long long' > Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5828618092937216 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/jpeg2000htdec.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c > index 3985783f3a..ae2ee6d6ee 100644 > --- a/libavcodec/jpeg2000htdec.c > +++ b/libavcodec/jpeg2000htdec.c > @@ -689,6 +689,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, > for (int i = 0; i < 4; i++) { > m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); > m[J2K_Q2][i] = sigma_n[4 * q2 + i] * U[J2K_Q2] - ((emb_pat_k[J2K_Q2] >> i) & 1); > + if (m[J2K_Q1][i] > 63 || m[J2K_Q2][i] > 63) { AFAIK, m[i], which is m_n in the standard, can never be larger than the sample bit depth (including the sign bit, if any). Is it worth comparing it to a value more precise than 63? > + ret = AVERROR_INVALIDDATA; > + goto free; > + } > } > > recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, > @@ -723,8 +727,13 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, > > U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1]; > > - for (int i = 0; i < 4; i++) > + for (int i = 0; i < 4; i++) { > m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); > + if (m[J2K_Q1][i] > 63) { > + ret = AVERROR_INVALIDDATA; > + goto free; > + } > + } > > recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, > E, mu_n, Dcup, Pcup, pLSB); > @@ -855,6 +864,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, > for (int i = 0; i < 4; i++) { > m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); > m[J2K_Q2][i] = sigma_n[4 * q2 + i] * U[J2K_Q2] - ((emb_pat_k[J2K_Q2] >> i) & 1); > + if (m[J2K_Q1][i] > 63 || m[J2K_Q2][i] > 63) { > + ret = AVERROR_INVALIDDATA; > + goto free; > + } > } > recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, > E, mu_n, Dcup, Pcup, pLSB); > @@ -920,8 +933,13 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, > > U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1]; > > - for (int i = 0; i < 4; i++) > + for (int i = 0; i < 4; i++) { > m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); > + if (m[J2K_Q1][i] > 63) { > + ret = AVERROR_INVALIDDATA; > + goto free; > + } > + } > > recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, > E, mu_n, Dcup, Pcup, pLSB); > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Fri, Aug 04, 2023 at 06:19:46PM -0700, Pierre-Anthony Lemieux wrote: > On Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > This also fixes assertion failures > > > > Fixes: shift exponent 95 is too large for 64-bit type 'unsigned long long' > > Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5828618092937216 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/jpeg2000htdec.c | 22 ++++++++++++++++++++-- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c > > index 3985783f3a..ae2ee6d6ee 100644 > > --- a/libavcodec/jpeg2000htdec.c > > +++ b/libavcodec/jpeg2000htdec.c > > @@ -689,6 +689,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, > > for (int i = 0; i < 4; i++) { > > m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); > > m[J2K_Q2][i] = sigma_n[4 * q2 + i] * U[J2K_Q2] - ((emb_pat_k[J2K_Q2] >> i) & 1); > > + if (m[J2K_Q1][i] > 63 || m[J2K_Q2][i] > 63) { > > AFAIK, m[i], which is m_n in the standard, can never be larger than > the sample bit depth (including the sign bit, if any). Is it worth > comparing it to a value more precise than 63? probably, yes I think you know the spec better than i do, so you can probably pick the tightest bound quicker ... can you submit a patch doing that ? thx [...]
On Sat, Aug 5, 2023 at 9:30 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Fri, Aug 04, 2023 at 06:19:46PM -0700, Pierre-Anthony Lemieux wrote: > > On Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer > > <michael@niedermayer.cc> wrote: > > > > > > This also fixes assertion failures > > > > > > Fixes: shift exponent 95 is too large for 64-bit type 'unsigned long long' > > > Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5828618092937216 > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavcodec/jpeg2000htdec.c | 22 ++++++++++++++++++++-- > > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c > > > index 3985783f3a..ae2ee6d6ee 100644 > > > --- a/libavcodec/jpeg2000htdec.c > > > +++ b/libavcodec/jpeg2000htdec.c > > > @@ -689,6 +689,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, > > > for (int i = 0; i < 4; i++) { > > > m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); > > > m[J2K_Q2][i] = sigma_n[4 * q2 + i] * U[J2K_Q2] - ((emb_pat_k[J2K_Q2] >> i) & 1); > > > + if (m[J2K_Q1][i] > 63 || m[J2K_Q2][i] > 63) { > > > > AFAIK, m[i], which is m_n in the standard, can never be larger than > > the sample bit depth (including the sign bit, if any). Is it worth > > comparing it to a value more precise than 63? > > probably, yes > I think you know the spec better than i do, so you can probably pick > the tightest bound quicker ... > can you submit a patch doing that ? I plan to do so before week's end. > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The greatest way to live with honor in this world is to be what we pretend > to be. -- Socrates > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Sun, Aug 6, 2023 at 9:28 AM Pierre-Anthony Lemieux <pal@sandflow.com> wrote: > > On Sat, Aug 5, 2023 at 9:30 AM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > On Fri, Aug 04, 2023 at 06:19:46PM -0700, Pierre-Anthony Lemieux wrote: > > > On Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer > > > <michael@niedermayer.cc> wrote: > > > > > > > > This also fixes assertion failures > > > > > > > > Fixes: shift exponent 95 is too large for 64-bit type 'unsigned long long' > > > > Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5828618092937216 > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavcodec/jpeg2000htdec.c | 22 ++++++++++++++++++++-- > > > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c > > > > index 3985783f3a..ae2ee6d6ee 100644 > > > > --- a/libavcodec/jpeg2000htdec.c > > > > +++ b/libavcodec/jpeg2000htdec.c > > > > @@ -689,6 +689,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, > > > > for (int i = 0; i < 4; i++) { > > > > m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); > > > > m[J2K_Q2][i] = sigma_n[4 * q2 + i] * U[J2K_Q2] - ((emb_pat_k[J2K_Q2] >> i) & 1); > > > > + if (m[J2K_Q1][i] > 63 || m[J2K_Q2][i] > 63) { > > > > > > AFAIK, m[i], which is m_n in the standard, can never be larger than > > > the sample bit depth (including the sign bit, if any). Is it worth > > > comparing it to a value more precise than 63? > > > > probably, yes > > I think you know the spec better than i do, so you can probably pick > > the tightest bound quicker ... > > can you submit a patch doing that ? > > I plan to do so before week's end. https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230810234856.2636-1-pal@sandflow.com/ > > > > > thx > > > > [...] > > -- > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > > > The greatest way to live with honor in this world is to be what we pretend > > to be. -- Socrates > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c index 3985783f3a..ae2ee6d6ee 100644 --- a/libavcodec/jpeg2000htdec.c +++ b/libavcodec/jpeg2000htdec.c @@ -689,6 +689,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, for (int i = 0; i < 4; i++) { m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); m[J2K_Q2][i] = sigma_n[4 * q2 + i] * U[J2K_Q2] - ((emb_pat_k[J2K_Q2] >> i) & 1); + if (m[J2K_Q1][i] > 63 || m[J2K_Q2][i] > 63) { + ret = AVERROR_INVALIDDATA; + goto free; + } } recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, @@ -723,8 +727,13 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1]; - for (int i = 0; i < 4; i++) + for (int i = 0; i < 4; i++) { m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); + if (m[J2K_Q1][i] > 63) { + ret = AVERROR_INVALIDDATA; + goto free; + } + } recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, E, mu_n, Dcup, Pcup, pLSB); @@ -855,6 +864,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, for (int i = 0; i < 4; i++) { m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); m[J2K_Q2][i] = sigma_n[4 * q2 + i] * U[J2K_Q2] - ((emb_pat_k[J2K_Q2] >> i) & 1); + if (m[J2K_Q1][i] > 63 || m[J2K_Q2][i] > 63) { + ret = AVERROR_INVALIDDATA; + goto free; + } } recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, E, mu_n, Dcup, Pcup, pLSB); @@ -920,8 +933,13 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s, U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1]; - for (int i = 0; i < 4; i++) + for (int i = 0; i < 4; i++) { m[J2K_Q1][i] = sigma_n[4 * q1 + i] * U[J2K_Q1] - ((emb_pat_k[J2K_Q1] >> i) & 1); + if (m[J2K_Q1][i] > 63) { + ret = AVERROR_INVALIDDATA; + goto free; + } + } recover_mag_sgn(mag_sgn_stream, J2K_Q1, q1, m_n, known_1, emb_pat_1, v, m, E, mu_n, Dcup, Pcup, pLSB);
This also fixes assertion failures Fixes: shift exponent 95 is too large for 64-bit type 'unsigned long long' Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5828618092937216 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/jpeg2000htdec.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)