diff mbox series

[FFmpeg-devel,v3,1/3] libavcodec/jpeg2000dec: Fix codeblock decode check

Message ID 20200721180715.19237-1-gautamramk@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v3,1/3] libavcodec/jpeg2000dec: Fix codeblock decode check
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Gautam Ramakrishnan July 21, 2020, 6:07 p.m. UTC
From: Gautam Ramakrishnan <gautamramk@gmail.com>

The codeblock decoder checks whether the mqc decoder
has decoded the right number of bytes. However, this
check does not account for the fact that the mqc encoder's
flush routine adds 2 bytes of data which does not have to be
read by the decoder. The check is modified to account for
this. This patch solves issue #4827
---
 libavcodec/jpeg2000dec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Michael Niedermayer July 23, 2020, 2:26 p.m. UTC | #1
On Tue, Jul 21, 2020 at 11:37:13PM +0530, gautamramk@gmail.com wrote:
> From: Gautam Ramakrishnan <gautamramk@gmail.com>
> 
> The codeblock decoder checks whether the mqc decoder
> has decoded the right number of bytes. However, this
> check does not account for the fact that the mqc encoder's
> flush routine adds 2 bytes of data which does not have to be
> read by the decoder. The check is modified to account for
> this. This patch solves issue #4827
> ---
>  libavcodec/jpeg2000dec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index f5ad8402fb..5e9e97eb6a 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -1753,9 +1753,9 @@ static int decode_cblk(Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *codsty,
>          pass_cnt ++;
>      }
>  
> -    if (cblk->data + cblk->length - 2*(term_cnt < cblk->nb_terminations) != t1->mqc.bp) {
> +    if (cblk->data + cblk->length - 2 > t1->mqc.bp) {
>          av_log(s->avctx, AV_LOG_WARNING, "End mismatch %"PTRDIFF_SPECIFIER"\n",

This changes a equals check to a larger than check, so it removes checking
in one direction. The commit message doesnt explain this sufficiently i think

thx

[...]
Gautam Ramakrishnan July 23, 2020, 4:45 p.m. UTC | #2
On Thu, Jul 23, 2020 at 7:56 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Tue, Jul 21, 2020 at 11:37:13PM +0530, gautamramk@gmail.com wrote:
> > From: Gautam Ramakrishnan <gautamramk@gmail.com>
> >
> > The codeblock decoder checks whether the mqc decoder
> > has decoded the right number of bytes. However, this
> > check does not account for the fact that the mqc encoder's
> > flush routine adds 2 bytes of data which does not have to be
> > read by the decoder. The check is modified to account for
> > this. This patch solves issue #4827
> > ---
> >  libavcodec/jpeg2000dec.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > index f5ad8402fb..5e9e97eb6a 100644
> > --- a/libavcodec/jpeg2000dec.c
> > +++ b/libavcodec/jpeg2000dec.c
> > @@ -1753,9 +1753,9 @@ static int decode_cblk(Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *codsty,
> >          pass_cnt ++;
> >      }
> >
> > -    if (cblk->data + cblk->length - 2*(term_cnt < cblk->nb_terminations) != t1->mqc.bp) {
> > +    if (cblk->data + cblk->length - 2 > t1->mqc.bp) {
> >          av_log(s->avctx, AV_LOG_WARNING, "End mismatch %"PTRDIFF_SPECIFIER"\n",
>
> This changes a equals check to a larger than check, so it removes checking
> in one direction. The commit message doesnt explain this sufficiently i think
>
I think a good solution would be to add a check in the other direction
and warn when the added
0xFF bytes are read.
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Concerning the gods, I have no means of knowing whether they exist or not
> or of what sort they may be, because of the obscurity of the subject, and
> the brevity of human life -- Protagoras
> _______________________________________________
> 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/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index f5ad8402fb..5e9e97eb6a 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -1753,9 +1753,9 @@  static int decode_cblk(Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *codsty,
         pass_cnt ++;
     }
 
-    if (cblk->data + cblk->length - 2*(term_cnt < cblk->nb_terminations) != t1->mqc.bp) {
+    if (cblk->data + cblk->length - 2 > t1->mqc.bp) {
         av_log(s->avctx, AV_LOG_WARNING, "End mismatch %"PTRDIFF_SPECIFIER"\n",
-               cblk->data + cblk->length - 2*(term_cnt < cblk->nb_terminations) - t1->mqc.bp);
+               cblk->data + cblk->length - 2 - t1->mqc.bp);
     }
 
     return 1;