[FFmpeg-devel] [PATCH 1/3] avcodec/jpeg2000htdec: Check magp before using it in a shift

[FFmpeg-devel] [PATCH 1/3] avcodec/jpeg2000htdec: Check magp before using it in a shift

[Michael Niedermayer]

Fixes: shift exponent -1 is negative
Fixes: 65378/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5457678193197056

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/jpeg2000dec.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 1afc6b1e2dd..fe2afb05057 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -1910,6 +1910,8 @@ static inline void tile_codeblocks(const Jpeg2000DecoderContext *s, Jpeg2000Tile
                 int nb_precincts, precno;
                 Jpeg2000Band *band = rlevel->band + bandno;
                 int cblkno = 0, bandpos;
+                /* See Rec. ITU-T T.800, Equation E-2 */
+                int magp = quantsty->expn[subbandno] + quantsty->nguardbits - 1;
 
                 bandpos = bandno + (reslevelno > 0);
 
@@ -1917,6 +1919,9 @@ static inline void tile_codeblocks(const Jpeg2000DecoderContext *s, Jpeg2000Tile
                     band->coord[1][0] == band->coord[1][1])
                     continue;
 
+                if ((codsty->cblk_style & JPEG2000_CTSY_HTJ2K_F) && magp >= 31)
+                    return;
+
                 nb_precincts = rlevel->num_precincts_x * rlevel->num_precincts_y;
                 /* Loop on precincts */
                 for (precno = 0; precno < nb_precincts; precno++) {
@@ -1927,8 +1932,6 @@ static inline void tile_codeblocks(const Jpeg2000DecoderContext *s, Jpeg2000Tile
                          cblkno < prec->nb_codeblocks_width * prec->nb_codeblocks_height;
                          cblkno++) {
                         int x, y, ret;
-                        /* See Rec. ITU-T T.800, Equation E-2 */
-                        int magp = quantsty->expn[subbandno] + quantsty->nguardbits - 1;
 
                         Jpeg2000Cblk *cblk = prec->cblk + cblkno;
 
-- 
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".

[FFmpeg-devel] [PATCH 2/3] avcodec/jpeg2000htdec: warn about non zero roi shift

[Michael Niedermayer]

Suggested-by: Tomas Härdin <git@haerdin.se>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/jpeg2000htdec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
index 6b9898d3ff2..4f0b10b4293 100644
--- a/libavcodec/jpeg2000htdec.c
+++ b/libavcodec/jpeg2000htdec.c
@@ -1198,6 +1198,9 @@ ff_jpeg2000_decode_htj2k(const Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
     av_assert0(width * height <= 4096);
     av_assert0(width * height > 0);
 
+    if (roi_shift)
+        avpriv_report_missing_feature(s->avctx, "ROI shift");
+
     memset(t1->data, 0, t1->stride * height * sizeof(*t1->data));
     memset(t1->flags, 0, t1->stride * (height + 2) * sizeof(*t1->flags));
 
-- 
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".

[FFmpeg-devel] [PATCH 3/3] avformat/mxfdec: Check first case of offset_temp computation for overflow

[Michael Niedermayer]

This is kind of ugly
Fixes: signed integer overflow: 255 * 1157565362826411919 cannot be represented in type 'long'
Fixes: 67313/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-6250434245230592

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mxfdec.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index c9af4628555..fe86f516630 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1891,9 +1891,14 @@ static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t
         if (edit_unit < s->index_start_position + s->index_duration) {
             int64_t index = edit_unit - s->index_start_position;
 
-            if (s->edit_unit_byte_count)
+            if (s->edit_unit_byte_count) {
+                if (s->edit_unit_byte_count * (uint64_t)index / s->edit_unit_byte_count != index ||
+                    s->edit_unit_byte_count * index > INT64_MAX - offset_temp
+                )
+                    return AVERROR_INVALIDDATA;
+
                 offset_temp += s->edit_unit_byte_count * index;
-            else {
+            } else {
                 if (s->nb_index_entries == 2 * s->index_duration + 1)
                     index *= 2;     /* Avid index */
 
-- 
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".

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/jpeg2000htdec: Check magp before using it in a shift

[Tomas Härdin]

fre 2024-03-29 klockan 20:32 +0100 skrev Michael Niedermayer:
> Fixes: shift exponent -1 is negative
> Fixes: 65378/clusterfuzz-testcase-minimized-
> ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5457678193197056
> 
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/jpeg2000dec.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index 1afc6b1e2dd..fe2afb05057 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -1910,6 +1910,8 @@ static inline void tile_codeblocks(const
> Jpeg2000DecoderContext *s, Jpeg2000Tile
>                  int nb_precincts, precno;
>                  Jpeg2000Band *band = rlevel->band + bandno;
>                  int cblkno = 0, bandpos;
> +                /* See Rec. ITU-T T.800, Equation E-2 */
> +                int magp = quantsty->expn[subbandno] + quantsty-
> >nguardbits - 1;
>  
>                  bandpos = bandno + (reslevelno > 0);
>  
> @@ -1917,6 +1919,9 @@ static inline void tile_codeblocks(const
> Jpeg2000DecoderContext *s, Jpeg2000Tile
>                      band->coord[1][0] == band->coord[1][1])
>                      continue;
>  
> +                if ((codsty->cblk_style & JPEG2000_CTSY_HTJ2K_F) &&
> magp >= 31)
> +                    return;

Please also print an error message and return AVERROR_PATCHWELCOME

/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/mxfdec: Check first case of offset_temp computation for overflow

[Tomas Härdin]

fre 2024-03-29 klockan 20:32 +0100 skrev Michael Niedermayer:
> This is kind of ugly
> Fixes: signed integer overflow: 255 * 1157565362826411919 cannot be
> represented in type 'long'
> Fixes: 67313/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> 6250434245230592
> 
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mxfdec.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index c9af4628555..fe86f516630 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -1891,9 +1891,14 @@ static int
> mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t
>          if (edit_unit < s->index_start_position + s->index_duration)
> {
>              int64_t index = edit_unit - s->index_start_position;
>  
> -            if (s->edit_unit_byte_count)
> +            if (s->edit_unit_byte_count) {
> +                if (s->edit_unit_byte_count * (uint64_t)index / s-
> >edit_unit_byte_count != index ||

Don't we already have a function for testing these kinds of overflows
for av_calloc()? Or do it manually less uglily like so:

  index > INT64_MAX / s->edit_unit_byte_count

> +                    s->edit_unit_byte_count * index > INT64_MAX -
> offset_temp
> +                )

Nit: looks a bit weird to have the ) there rather than at the end of
the previous line


/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/jpeg2000htdec: Check magp before using it in a shift

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2032 bytes --]

On Sat, Mar 30, 2024 at 09:56:58AM +0100, Tomas Härdin wrote:
> fre 2024-03-29 klockan 20:32 +0100 skrev Michael Niedermayer:
> > Fixes: shift exponent -1 is negative
> > Fixes: 65378/clusterfuzz-testcase-minimized-
> > ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5457678193197056
> > 
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/jpeg2000dec.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > index 1afc6b1e2dd..fe2afb05057 100644
> > --- a/libavcodec/jpeg2000dec.c
> > +++ b/libavcodec/jpeg2000dec.c
> > @@ -1910,6 +1910,8 @@ static inline void tile_codeblocks(const
> > Jpeg2000DecoderContext *s, Jpeg2000Tile
> >                  int nb_precincts, precno;
> >                  Jpeg2000Band *band = rlevel->band + bandno;
> >                  int cblkno = 0, bandpos;
> > +                /* See Rec. ITU-T T.800, Equation E-2 */
> > +                int magp = quantsty->expn[subbandno] + quantsty-
> > >nguardbits - 1;
> >  
> >                  bandpos = bandno + (reslevelno > 0);
> >  
> > @@ -1917,6 +1919,9 @@ static inline void tile_codeblocks(const
> > Jpeg2000DecoderContext *s, Jpeg2000Tile
> >                      band->coord[1][0] == band->coord[1][1])
> >                      continue;
> >  
> > +                if ((codsty->cblk_style & JPEG2000_CTSY_HTJ2K_F) &&
> > magp >= 31)
> > +                    return;
> 
> Please also print an error message and return AVERROR_PATCHWELCOME

will apply with these changes

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/mxfdec: Check first case of offset_temp computation for overflow

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2098 bytes --]

On Sat, Mar 30, 2024 at 10:01:32AM +0100, Tomas Härdin wrote:
> fre 2024-03-29 klockan 20:32 +0100 skrev Michael Niedermayer:
> > This is kind of ugly
> > Fixes: signed integer overflow: 255 * 1157565362826411919 cannot be
> > represented in type 'long'
> > Fixes: 67313/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> > 6250434245230592
> > 
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/mxfdec.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index c9af4628555..fe86f516630 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -1891,9 +1891,14 @@ static int
> > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t
> >          if (edit_unit < s->index_start_position + s->index_duration)
> > {
> >              int64_t index = edit_unit - s->index_start_position;
> >  
> > -            if (s->edit_unit_byte_count)
> > +            if (s->edit_unit_byte_count) {
> > +                if (s->edit_unit_byte_count * (uint64_t)index / s-
> > >edit_unit_byte_count != index ||
> 
> Don't we already have a function for testing these kinds of overflows

We have av_size_mult() thats for size_t


> for av_calloc()? Or do it manually less uglily like so:
> 
>   index > INT64_MAX / s->edit_unit_byte_count

ok

> 
> > +                    s->edit_unit_byte_count * index > INT64_MAX -
> > offset_temp
> > +                )
> 
> Nit: looks a bit weird to have the ) there rather than at the end of
> the previous line

will push with this changed

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
than the original author, trying to rewrite it will not make it better.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".