[FFmpeg-devel] [PATCH 1/3] avcodec/jpeg2000htdec: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k()

[FFmpeg-devel] [PATCH 1/3] avcodec/jpeg2000htdec: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k()

[Michael Niedermayer]

Fixes: freeing of uninitialized pointers
Fixes: part of 58299

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
index 4c4e54710d..2b082b3b2f 100644
--- a/libavcodec/jpeg2000htdec.c
+++ b/libavcodec/jpeg2000htdec.c
@@ -1174,8 +1174,8 @@ ff_jpeg2000_decode_htj2k(const Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
     int ret;
 
     /* Temporary buffers */
-    int32_t *sample_buf;
-    uint8_t *block_states;
+    int32_t *sample_buf = NULL;
+    uint8_t *block_states = NULL;
 
     int32_t n, val;             // Post-processing
 
-- 
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: Consolidate jpeg2000 spec bits in jpeg2000_bitbuf_refill_backwards()

[Michael Niedermayer]

Code should make more sense now

Fixes: out of array access
Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-6627570448465920

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 | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
index 2b082b3b2f..3985783f3a 100644
--- a/libavcodec/jpeg2000htdec.c
+++ b/libavcodec/jpeg2000htdec.c
@@ -159,21 +159,14 @@ static int jpeg2000_bitbuf_refill_backwards(StateVars *buffer, const uint8_t *ar
      */
     position -= 4;
 
-    tmp = AV_RB32(&array[position + 1]);
-
-    if (buffer->pos < 4){
-        /* mask un-needed bits if we are close to input end */
-        uint64_t mask = (1ull << (buffer->pos + 1) * 8) - 1;
-        tmp &= mask;
-    }
-
     /**
      *  Unstuff bits. Load a temporary byte, which precedes the position we
      *  currently at, to ensure that we can also un-stuff if the stuffed bit is
      *  the bottom most bits.
      */
-    tmp <<= 8;
-    tmp |= array[buffer->pos + 1];
+
+    for(int i = FFMAX(0, position + 1); i <= buffer->pos + 1; i++)
+        tmp = 256*tmp + array[i];
 
     if ((tmp & 0x7FFF000000) > 0x7F8F000000) {
         tmp &= 0x7FFFFFFFFF;
-- 
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] avcodec/jpeg2000htdec: Check m

[Michael Niedermayer]

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) {
+                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".

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/jpeg2000htdec: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k()

[Tomas Härdin]

ons 2023-08-02 klockan 02:01 +0200 skrev Michael Niedermayer:
> Fixes: freeing of uninitialized pointers
> Fixes: part of 58299
> 
> 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 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> index 4c4e54710d..2b082b3b2f 100644
> --- a/libavcodec/jpeg2000htdec.c
> +++ b/libavcodec/jpeg2000htdec.c
> @@ -1174,8 +1174,8 @@ ff_jpeg2000_decode_htj2k(const
> Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
>      int ret;
>  
>      /* Temporary buffers */
> -    int32_t *sample_buf;
> -    uint8_t *block_states;
> +    int32_t *sample_buf = NULL;
> +    uint8_t *block_states = NULL;

Looks OK

/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: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k()

[Michael Niedermayer]

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

On Thu, Aug 03, 2023 at 10:20:29AM +0200, Tomas Härdin wrote:
> ons 2023-08-02 klockan 02:01 +0200 skrev Michael Niedermayer:
> > Fixes: freeing of uninitialized pointers
> > Fixes: part of 58299
> > 
> > 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 | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> > index 4c4e54710d..2b082b3b2f 100644
> > --- a/libavcodec/jpeg2000htdec.c
> > +++ b/libavcodec/jpeg2000htdec.c
> > @@ -1174,8 +1174,8 @@ ff_jpeg2000_decode_htj2k(const
> > Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
> >      int ret;
> >  
> >      /* Temporary buffers */
> > -    int32_t *sample_buf;
> > -    uint8_t *block_states;
> > +    int32_t *sample_buf = NULL;
> > +    uint8_t *block_states = NULL;
> 
> Looks OK

will apply

thx

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

No snowflake in an avalanche ever feels responsible. -- Voltaire

[-- 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 1/3] avcodec/jpeg2000htdec: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k()

[Tomas Härdin]

tor 2023-08-03 klockan 17:36 +0200 skrev Michael Niedermayer:
> On Thu, Aug 03, 2023 at 10:20:29AM +0200, Tomas Härdin wrote:
> > ons 2023-08-02 klockan 02:01 +0200 skrev Michael Niedermayer:
> > > Fixes: freeing of uninitialized pointers
> > > Fixes: part of 58299
> > > 
> > > 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 | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/libavcodec/jpeg2000htdec.c
> > > b/libavcodec/jpeg2000htdec.c
> > > index 4c4e54710d..2b082b3b2f 100644
> > > --- a/libavcodec/jpeg2000htdec.c
> > > +++ b/libavcodec/jpeg2000htdec.c
> > > @@ -1174,8 +1174,8 @@ ff_jpeg2000_decode_htj2k(const
> > > Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
> > >      int ret;
> > >  
> > >      /* Temporary buffers */
> > > -    int32_t *sample_buf;
> > > -    uint8_t *block_states;
> > > +    int32_t *sample_buf = NULL;
> > > +    uint8_t *block_states = NULL;
> > 
> > Looks OK
> 
> will apply

I should add that I don't know if the two other patches are fine

/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: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k()

[Pierre-Anthony Lemieux]

On Thu, Aug 3, 2023 at 1:50 PM Tomas Härdin <git@haerdin.se> wrote:
>
> tor 2023-08-03 klockan 17:36 +0200 skrev Michael Niedermayer:
> > On Thu, Aug 03, 2023 at 10:20:29AM +0200, Tomas Härdin wrote:
> > > ons 2023-08-02 klockan 02:01 +0200 skrev Michael Niedermayer:
> > > > Fixes: freeing of uninitialized pointers
> > > > Fixes: part of 58299
> > > >
> > > > 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 | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/libavcodec/jpeg2000htdec.c
> > > > b/libavcodec/jpeg2000htdec.c
> > > > index 4c4e54710d..2b082b3b2f 100644
> > > > --- a/libavcodec/jpeg2000htdec.c
> > > > +++ b/libavcodec/jpeg2000htdec.c
> > > > @@ -1174,8 +1174,8 @@ ff_jpeg2000_decode_htj2k(const
> > > > Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
> > > >      int ret;
> > > >
> > > >      /* Temporary buffers */
> > > > -    int32_t *sample_buf;
> > > > -    uint8_t *block_states;
> > > > +    int32_t *sample_buf = NULL;
> > > > +    uint8_t *block_states = NULL;
> > >
> > > Looks OK
> >
> > will apply
>
> I should add that I don't know if the two other patches are fine

I am working on them.

>
> /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".
_______________________________________________
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: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k()

[Caleb Etemesi]

Looks good, for the other two, I may need to check with a problematic
sample I had to see if it works

Kind regards,
Caleb Etemesi

On Thu, 3 Aug 2023, 23:58 Pierre-Anthony Lemieux, <pal@sandflow.com> wrote:

> On Thu, Aug 3, 2023 at 1:50 PM Tomas Härdin <git@haerdin.se> wrote:
> >
> > tor 2023-08-03 klockan 17:36 +0200 skrev Michael Niedermayer:
> > > On Thu, Aug 03, 2023 at 10:20:29AM +0200, Tomas Härdin wrote:
> > > > ons 2023-08-02 klockan 02:01 +0200 skrev Michael Niedermayer:
> > > > > Fixes: freeing of uninitialized pointers
> > > > > Fixes: part of 58299
> > > > >
> > > > > 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 | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/libavcodec/jpeg2000htdec.c
> > > > > b/libavcodec/jpeg2000htdec.c
> > > > > index 4c4e54710d..2b082b3b2f 100644
> > > > > --- a/libavcodec/jpeg2000htdec.c
> > > > > +++ b/libavcodec/jpeg2000htdec.c
> > > > > @@ -1174,8 +1174,8 @@ ff_jpeg2000_decode_htj2k(const
> > > > > Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
> > > > >      int ret;
> > > > >
> > > > >      /* Temporary buffers */
> > > > > -    int32_t *sample_buf;
> > > > > -    uint8_t *block_states;
> > > > > +    int32_t *sample_buf = NULL;
> > > > > +    uint8_t *block_states = NULL;
> > > >
> > > > Looks OK
> > >
> > > will apply
> >
> > I should add that I don't know if the two other patches are fine
>
> I am working on them.
>
> >
> > /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".
> _______________________________________________
> 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 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] avcodec/jpeg2000htdec: Check m

[Pierre-Anthony Lemieux]

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".
_______________________________________________
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 2/3] avcodec/jpeg2000htdec: Consolidate jpeg2000 spec bits in jpeg2000_bitbuf_refill_backwards()

[Pierre-Anthony Lemieux]

On Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> Code should make more sense now
>
> Fixes: out of array access
> Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-6627570448465920
>
> 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 | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> index 2b082b3b2f..3985783f3a 100644
> --- a/libavcodec/jpeg2000htdec.c
> +++ b/libavcodec/jpeg2000htdec.c
> @@ -159,21 +159,14 @@ static int jpeg2000_bitbuf_refill_backwards(StateVars *buffer, const uint8_t *ar
>       */
>      position -= 4;

Can't we get rid of this line and the comment above, and instead
replace `int32_t position = buffer->pos;` with `int32_t position =
buffer->pos - 4;`?

LGTM otherwise.

>
> -    tmp = AV_RB32(&array[position + 1]);
> -
> -    if (buffer->pos < 4){
> -        /* mask un-needed bits if we are close to input end */
> -        uint64_t mask = (1ull << (buffer->pos + 1) * 8) - 1;
> -        tmp &= mask;
> -    }
> -
>      /**
>       *  Unstuff bits. Load a temporary byte, which precedes the position we
>       *  currently at, to ensure that we can also un-stuff if the stuffed bit is
>       *  the bottom most bits.
>       */
> -    tmp <<= 8;
> -    tmp |= array[buffer->pos + 1];
> +
> +    for(int i = FFMAX(0, position + 1); i <= buffer->pos + 1; i++)
> +        tmp = 256*tmp + array[i];
>
>      if ((tmp & 0x7FFF000000) > 0x7F8F000000) {
>          tmp &= 0x7FFFFFFFFF;
> --
> 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 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 2/3] avcodec/jpeg2000htdec: Consolidate jpeg2000 spec bits in jpeg2000_bitbuf_refill_backwards()

[Michael Niedermayer]

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

On Fri, Aug 04, 2023 at 06:41:24PM -0700, Pierre-Anthony Lemieux wrote:
> On Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > Code should make more sense now
> >
> > Fixes: out of array access
> > Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-6627570448465920
> >
> > 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 | 13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> > index 2b082b3b2f..3985783f3a 100644
> > --- a/libavcodec/jpeg2000htdec.c
> > +++ b/libavcodec/jpeg2000htdec.c
> > @@ -159,21 +159,14 @@ static int jpeg2000_bitbuf_refill_backwards(StateVars *buffer, const uint8_t *ar
> >       */
> >      position -= 4;
> 
> Can't we get rid of this line and the comment above, and instead
> replace `int32_t position = buffer->pos;` with `int32_t position =
> buffer->pos - 4;`?

yes


> 
> LGTM otherwise.

will apply with the suggested change

thx

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

It is a danger to trust the dream we wish for rather than
the science we have, -- Dr. Kenneth Brown

[-- 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] avcodec/jpeg2000htdec: Check m

[Michael Niedermayer]

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

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

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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates

[-- 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] avcodec/jpeg2000htdec: Check m

[Pierre-Anthony Lemieux]

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".
_______________________________________________
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] avcodec/jpeg2000htdec: Check m

[Pierre-Anthony Lemieux]

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