diff mbox series

[FFmpeg-devel,v1] avcodec/jpeg2000htdec: check if block decoding will exceed internal precision

Message ID 20230810234856.2636-1-pal@sandflow.com
State New
Headers show
Series [FFmpeg-devel,v1] avcodec/jpeg2000htdec: check if block decoding will exceed internal precision | expand

Checks

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

Commit Message

Pierre-Anthony Lemieux Aug. 10, 2023, 11:48 p.m. UTC
From: Pierre-Anthony Lemieux <pal@palemieux.com>

Intended to replace https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230802000135.26482-3-michael@niedermayer.cc/
with a more accurate block decoding magnitude bound.

---
 libavcodec/jpeg2000htdec.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer Aug. 12, 2023, 6:39 p.m. UTC | #1
On Thu, Aug 10, 2023 at 04:48:56PM -0700, pal@sandflow.com wrote:
> From: Pierre-Anthony Lemieux <pal@palemieux.com>
> 
> Intended to replace https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230802000135.26482-3-michael@niedermayer.cc/
> with a more accurate block decoding magnitude bound.
> 
> ---
>  libavcodec/jpeg2000htdec.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> index 2c4cea5dd9..713dba17e3 100644
> --- a/libavcodec/jpeg2000htdec.c
> +++ b/libavcodec/jpeg2000htdec.c
> @@ -572,6 +572,14 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
>  
>      const uint8_t *vlc_buf = Dcup + Pcup;
>  
> +    /*
> +     * Bound on the recision needed to process the codeblock. The number of
> +     * decoded bit planes is equal to at most cblk->zbp + 2 since S_blk = P if
> +     * there are no placeholder passes or HT Sets and P = cblk->zbp. See Rec.
> +     * ITU-T T.814, 7.6.
> +     */
> +    int maxbp = cblk->zbp + 2;
> +
>      /* convert to raster-scan */
>      const uint16_t is_border_x = width % 2;
>      const uint16_t is_border_y = height % 2;
> @@ -590,6 +598,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
>          goto free;
>      }
>  
> +    /* do we have enough precision, assuming a 32-bit decoding path */
> +    if (maxbp >= 32)
> +        return AVERROR_INVALIDDATA;

doesnt this need a goto free ?

thx

[...]
Pierre-Anthony Lemieux Aug. 12, 2023, 8:31 p.m. UTC | #2
On Sat, Aug 12, 2023 at 11:39 AM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Thu, Aug 10, 2023 at 04:48:56PM -0700, pal@sandflow.com wrote:
> > From: Pierre-Anthony Lemieux <pal@palemieux.com>
> >
> > Intended to replace https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230802000135.26482-3-michael@niedermayer.cc/
> > with a more accurate block decoding magnitude bound.
> >
> > ---
> >  libavcodec/jpeg2000htdec.c | 32 +++++++++++++++++++++++++++++++-
> >  1 file changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> > index 2c4cea5dd9..713dba17e3 100644
> > --- a/libavcodec/jpeg2000htdec.c
> > +++ b/libavcodec/jpeg2000htdec.c
> > @@ -572,6 +572,14 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
> >
> >      const uint8_t *vlc_buf = Dcup + Pcup;
> >
> > +    /*
> > +     * Bound on the recision needed to process the codeblock. The number of
> > +     * decoded bit planes is equal to at most cblk->zbp + 2 since S_blk = P if
> > +     * there are no placeholder passes or HT Sets and P = cblk->zbp. See Rec.
> > +     * ITU-T T.814, 7.6.
> > +     */
> > +    int maxbp = cblk->zbp + 2;
> > +
> >      /* convert to raster-scan */
> >      const uint16_t is_border_x = width % 2;
> >      const uint16_t is_border_y = height % 2;
> > @@ -590,6 +598,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
> >          goto free;
> >      }
> >
> > +    /* do we have enough precision, assuming a 32-bit decoding path */
> > +    if (maxbp >= 32)
> > +        return AVERROR_INVALIDDATA;
>
> doesnt this need a goto free ?

Yes. Just pushed v2 of the patchset.

>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No human being will ever know the Truth, for even if they happen to say it
> by chance, they would not even known they had done so. -- Xenophanes
> _______________________________________________
> 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".
Pierre-Anthony Lemieux Aug. 12, 2023, 8:32 p.m. UTC | #3
On Sat, Aug 12, 2023 at 1:31 PM Pierre-Anthony Lemieux <pal@sandflow.com> wrote:
>
> On Sat, Aug 12, 2023 at 11:39 AM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Thu, Aug 10, 2023 at 04:48:56PM -0700, pal@sandflow.com wrote:
> > > From: Pierre-Anthony Lemieux <pal@palemieux.com>
> > >
> > > Intended to replace https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230802000135.26482-3-michael@niedermayer.cc/
> > > with a more accurate block decoding magnitude bound.
> > >
> > > ---
> > >  libavcodec/jpeg2000htdec.c | 32 +++++++++++++++++++++++++++++++-
> > >  1 file changed, 31 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
> > > index 2c4cea5dd9..713dba17e3 100644
> > > --- a/libavcodec/jpeg2000htdec.c
> > > +++ b/libavcodec/jpeg2000htdec.c
> > > @@ -572,6 +572,14 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
> > >
> > >      const uint8_t *vlc_buf = Dcup + Pcup;
> > >
> > > +    /*
> > > +     * Bound on the recision needed to process the codeblock. The number of
> > > +     * decoded bit planes is equal to at most cblk->zbp + 2 since S_blk = P if
> > > +     * there are no placeholder passes or HT Sets and P = cblk->zbp. See Rec.
> > > +     * ITU-T T.814, 7.6.
> > > +     */
> > > +    int maxbp = cblk->zbp + 2;
> > > +
> > >      /* convert to raster-scan */
> > >      const uint16_t is_border_x = width % 2;
> > >      const uint16_t is_border_y = height % 2;
> > > @@ -590,6 +598,10 @@ static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
> > >          goto free;
> > >      }
> > >
> > > +    /* do we have enough precision, assuming a 32-bit decoding path */
> > > +    if (maxbp >= 32)
> > > +        return AVERROR_INVALIDDATA;
> >
> > doesnt this need a goto free ?
>
> Yes. Just pushed v2 of the patchset.

I meant "sent it to this reflector".

>
> >
> > thx
> >
> > [...]
> >
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > No human being will ever know the Truth, for even if they happen to say it
> > by chance, they would not even known they had done so. -- Xenophanes
> > _______________________________________________
> > 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 mbox series

Patch

diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c
index 2c4cea5dd9..713dba17e3 100644
--- a/libavcodec/jpeg2000htdec.c
+++ b/libavcodec/jpeg2000htdec.c
@@ -572,6 +572,14 @@  static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
 
     const uint8_t *vlc_buf = Dcup + Pcup;
 
+    /*
+     * Bound on the recision needed to process the codeblock. The number of
+     * decoded bit planes is equal to at most cblk->zbp + 2 since S_blk = P if
+     * there are no placeholder passes or HT Sets and P = cblk->zbp. See Rec.
+     * ITU-T T.814, 7.6.
+     */
+    int maxbp = cblk->zbp + 2;
+
     /* convert to raster-scan */
     const uint16_t is_border_x = width % 2;
     const uint16_t is_border_y = height % 2;
@@ -590,6 +598,10 @@  static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
         goto free;
     }
 
+    /* do we have enough precision, assuming a 32-bit decoding path */
+    if (maxbp >= 32)
+        return AVERROR_INVALIDDATA;
+
     sigma = sigma_n;
     mu = mu_n;
 
@@ -676,6 +688,10 @@  static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
         }
         U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1];
         U[J2K_Q2] = kappa[J2K_Q2] + u[J2K_Q2];
+        if (U[J2K_Q1] > maxbp || U[J2K_Q2] > maxbp) {
+            ret = AVERROR_INVALIDDATA;
+            goto free;
+        }
 
         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);
@@ -713,6 +729,10 @@  static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
         }
 
         U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1];
+        if (U[J2K_Q1] > maxbp) {
+            ret = AVERROR_INVALIDDATA;
+            goto free;
+        }
 
         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);
@@ -842,6 +862,10 @@  static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
 
             U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1];
             U[J2K_Q2] = kappa[J2K_Q2] + u[J2K_Q2];
+            if (U[J2K_Q1] > maxbp || U[J2K_Q2] > maxbp) {
+                ret = AVERROR_INVALIDDATA;
+                goto free;
+            }
 
             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);
@@ -910,6 +934,10 @@  static int jpeg2000_decode_ht_cleanup_segment(const Jpeg2000DecoderContext *s,
             kappa[J2K_Q1] = FFMAX(1, gamma[J2K_Q1] * (max_e[J2K_Q1] - 1));
 
             U[J2K_Q1] = kappa[J2K_Q1] + u[J2K_Q1];
+            if (U[J2K_Q1] > maxbp) {
+                ret = AVERROR_INVALIDDATA;
+                goto free;
+            }
 
             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);
@@ -1238,8 +1266,10 @@  ff_jpeg2000_decode_htj2k(const Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
     }
     if ((ret = jpeg2000_decode_ht_cleanup_segment(s, cblk, t1, &mel_state, &mel, &vlc,
                                           &mag_sgn, Dcup, Lcup, Pcup, pLSB, width,
-                                          height, sample_buf, block_states)) < 0)
+                                          height, sample_buf, block_states)) < 0) {
+        av_log(s->avctx, AV_LOG_ERROR, "Bad HT cleanup segment\n");
         goto free;
+    }
 
     if (cblk->npasses > 1)
         jpeg2000_decode_sigprop_segment(cblk, width, height, Dref, Lref,