diff mbox series

[FFmpeg-devel,1/2] avcodec/cdtoons: Correct several end of data checks in cdtoons_render_sprite()

Message ID 20200220185056.10900-1-michael@niedermayer.cc
State Accepted
Headers show
Series [FFmpeg-devel,1/2] avcodec/cdtoons: Correct several end of data checks in cdtoons_render_sprite()
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Michael Niedermayer Feb. 20, 2020, 6:50 p.m. UTC
No testcases, found by code review when debuging issue found by oss-fuzz

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/cdtoons.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

Paul B Mahol Feb. 20, 2020, 10:32 p.m. UTC | #1
LGTM

On 2/20/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> No testcases, found by code review when debuging issue found by oss-fuzz
>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/cdtoons.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/libavcodec/cdtoons.c b/libavcodec/cdtoons.c
> index 24a328352c..dc4fa6bf0b 100644
> --- a/libavcodec/cdtoons.c
> +++ b/libavcodec/cdtoons.c
> @@ -82,9 +82,11 @@ static int cdtoons_render_sprite(AVCodecContext *avctx,
> const uint8_t *data,
>      for (int y = 0; y < height; y++) {
>          /* one scanline at a time, size is provided */
>          data      = next_line;
> -        if (data > end - 2)
> +        if (end - data < 2)
>              return 1;
>          line_size = bytestream_get_be16(&data);
> +        if (end - data < line_size)
> +            return 1;
>          next_line = data + line_size;
>          if (dst_y + y < 0)
>              continue;
> @@ -94,7 +96,7 @@ static int cdtoons_render_sprite(AVCodecContext *avctx,
> const uint8_t *data,
>          to_skip = skip;
>          x       = 0;
>          while (x < width - skip) {
> -            int raw, size;
> +            int raw, size, step;
>              uint8_t val;
>
>              if (data >= end)
> @@ -108,20 +110,22 @@ static int cdtoons_render_sprite(AVCodecContext
> *avctx, const uint8_t *data,
>              if (to_skip >= size) {
>                  to_skip -= size;
>                  if (raw) {
> -                    data += size;
> +                    step = size;
>                  } else {
> -                    data += 1;
> +                    step = 1;
>                  }
> -                if (data > next_line)
> +                if (next_line - data < step)
>                      return 1;
> +                data += step;
>                  continue;
>              } else if (to_skip) {
>                  size -= to_skip;
> -                if (raw)
> +                if (raw) {
> +                    if (next_line - data < to_skip)
> +                        return 1;
>                      data += to_skip;
> +                }
>                  to_skip = 0;
> -                if (data > next_line)
> -                    return 1;
>              }
>
>              if (x + size >= width - skip)
> @@ -129,10 +133,10 @@ static int cdtoons_render_sprite(AVCodecContext
> *avctx, const uint8_t *data,
>
>              /* either raw data, or a run of a single color */
>              if (raw) {
> +                if (next_line - data < size)
> +                    return 1;
>                  memcpy(dest + x, data, size);
>                  data += size;
> -                if (data > next_line)
> -                    return 1;
>              } else {
>                  uint8_t color = bytestream_get_byte(&data);
>                  /* ignore transparent runs */
> --
> 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".
Michael Niedermayer Feb. 21, 2020, 8:38 p.m. UTC | #2
On Thu, Feb 20, 2020 at 11:32:08PM +0100, Paul B Mahol wrote:
> LGTM

will apply

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/cdtoons.c b/libavcodec/cdtoons.c
index 24a328352c..dc4fa6bf0b 100644
--- a/libavcodec/cdtoons.c
+++ b/libavcodec/cdtoons.c
@@ -82,9 +82,11 @@  static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data,
     for (int y = 0; y < height; y++) {
         /* one scanline at a time, size is provided */
         data      = next_line;
-        if (data > end - 2)
+        if (end - data < 2)
             return 1;
         line_size = bytestream_get_be16(&data);
+        if (end - data < line_size)
+            return 1;
         next_line = data + line_size;
         if (dst_y + y < 0)
             continue;
@@ -94,7 +96,7 @@  static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data,
         to_skip = skip;
         x       = 0;
         while (x < width - skip) {
-            int raw, size;
+            int raw, size, step;
             uint8_t val;
 
             if (data >= end)
@@ -108,20 +110,22 @@  static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data,
             if (to_skip >= size) {
                 to_skip -= size;
                 if (raw) {
-                    data += size;
+                    step = size;
                 } else {
-                    data += 1;
+                    step = 1;
                 }
-                if (data > next_line)
+                if (next_line - data < step)
                     return 1;
+                data += step;
                 continue;
             } else if (to_skip) {
                 size -= to_skip;
-                if (raw)
+                if (raw) {
+                    if (next_line - data < to_skip)
+                        return 1;
                     data += to_skip;
+                }
                 to_skip = 0;
-                if (data > next_line)
-                    return 1;
             }
 
             if (x + size >= width - skip)
@@ -129,10 +133,10 @@  static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data,
 
             /* either raw data, or a run of a single color */
             if (raw) {
+                if (next_line - data < size)
+                    return 1;
                 memcpy(dest + x, data, size);
                 data += size;
-                if (data > next_line)
-                    return 1;
             } else {
                 uint8_t color = bytestream_get_byte(&data);
                 /* ignore transparent runs */