[FFmpeg-devel] avcodec/videodsp_template: Fix overflow of addition

Submitted by Michael Niedermayer on June 18, 2019, 2:25 p.m.

Details

Message ID 20190618142547.952-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer June 18, 2019, 2:25 p.m.
Fixes: addition of unsigned offset to 0x7f56fc26a9b6 overflowed to 0x7f56fc26a8be*
Fixes: clusterfuzz-testcase-minimized-mediasource_MP4_AVC1_pipeline_integration_fuzzer-4917949056679936

Reported-by: Matt Wolenetz <wolenetz@google.com>
Reviewed-by: Matt Wolenetz <wolenetz@google.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/videodsp_template.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Reimar Döffinger June 21, 2019, 7:07 a.m.
On 18.06.2019, at 16:25, Michael Niedermayer <michael@niedermayer.cc> wrote:

> Fixes: addition of unsigned offset to 0x7f56fc26a9b6 overflowed to 0x7f56fc26a8be*
> Fixes: clusterfuzz-testcase-minimized-mediasource_MP4_AVC1_pipeline_integration_fuzzer-4917949056679936
> 
> Reported-by: Matt Wolenetz <wolenetz@google.com>
> Reviewed-by: Matt Wolenetz <wolenetz@google.com>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/videodsp_template.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/videodsp_template.c b/libavcodec/videodsp_template.c
> index 94c1b7188d..eae2f1d51b 100644
> --- a/libavcodec/videodsp_template.c
> +++ b/libavcodec/videodsp_template.c
> @@ -44,7 +44,7 @@ void FUNC(ff_emulated_edge_mc)(uint8_t *buf, const uint8_t *src,
>         src_y = 1 - block_h;
>     }
>     if (src_x >= w) {
> -        src  += (w - 1 - src_x) * sizeof(pixel);
> +        src  -= (1 + src_x - w) * sizeof(pixel);

This is really non-obvious and someone might be tempted to "simplify", especially since the old way matched the code a few lines below.
I'd suggest adding a comment.

>         src_x = w - 1;
>     } else if (src_x <= -block_w) {
>         src  += (1 - block_w - src_x) * sizeof(pixel);
Michael Niedermayer June 26, 2019, 7:41 p.m.
On Fri, Jun 21, 2019 at 09:07:38AM +0200, Reimar Döffinger wrote:
> 
> 
> On 18.06.2019, at 16:25, Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> > Fixes: addition of unsigned offset to 0x7f56fc26a9b6 overflowed to 0x7f56fc26a8be*
> > Fixes: clusterfuzz-testcase-minimized-mediasource_MP4_AVC1_pipeline_integration_fuzzer-4917949056679936
> > 
> > Reported-by: Matt Wolenetz <wolenetz@google.com>
> > Reviewed-by: Matt Wolenetz <wolenetz@google.com>
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavcodec/videodsp_template.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/videodsp_template.c b/libavcodec/videodsp_template.c
> > index 94c1b7188d..eae2f1d51b 100644
> > --- a/libavcodec/videodsp_template.c
> > +++ b/libavcodec/videodsp_template.c
> > @@ -44,7 +44,7 @@ void FUNC(ff_emulated_edge_mc)(uint8_t *buf, const uint8_t *src,
> >         src_y = 1 - block_h;
> >     }
> >     if (src_x >= w) {
> > -        src  += (w - 1 - src_x) * sizeof(pixel);
> > +        src  -= (1 + src_x - w) * sizeof(pixel);
> 
> This is really non-obvious and someone might be tempted to "simplify", especially since the old way matched the code a few lines below.
> I'd suggest adding a comment.

will apply with a comment

thanks


[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/videodsp_template.c b/libavcodec/videodsp_template.c
index 94c1b7188d..eae2f1d51b 100644
--- a/libavcodec/videodsp_template.c
+++ b/libavcodec/videodsp_template.c
@@ -44,7 +44,7 @@  void FUNC(ff_emulated_edge_mc)(uint8_t *buf, const uint8_t *src,
         src_y = 1 - block_h;
     }
     if (src_x >= w) {
-        src  += (w - 1 - src_x) * sizeof(pixel);
+        src  -= (1 + src_x - w) * sizeof(pixel);
         src_x = w - 1;
     } else if (src_x <= -block_w) {
         src  += (1 - block_w - src_x) * sizeof(pixel);