diff mbox series

[FFmpeg-devel,1/2] avcodec/bitstream: Check code length before truncating to uint8_t

Message ID 20201024130300.45644-1-andreas.rheinhardt@gmail.com
State Accepted
Commit e75b6ec43b63c5ba5ca2202ffd06df4d5a018e4a
Headers show
Series [FFmpeg-devel,1/2] avcodec/bitstream: Check code length before truncating to uint8_t
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Oct. 24, 2020, 1:02 p.m. UTC
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
The main motivation for this patch is actually the second patch and not
the improved check.

 libavcodec/bitstream.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Andreas Rheinhardt Oct. 27, 2020, 9:17 a.m. UTC | #1
Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> The main motivation for this patch is actually the second patch and not
> the improved check.
> 
>  libavcodec/bitstream.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c
> index 77c2b9ce05..ffa352b28b 100644
> --- a/libavcodec/bitstream.c
> +++ b/libavcodec/bitstream.c
> @@ -302,15 +302,17 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int nb_codes,
>      j = 0;
>  #define COPY(condition)\
>      for (i = 0; i < nb_codes; i++) {                                        \
> -        GET_DATA(buf[j].bits, bits, i, bits_wrap, bits_size);               \
> +        unsigned len;                                                       \
> +        GET_DATA(len, bits, i, bits_wrap, bits_size);                       \
>          if (!(condition))                                                   \
>              continue;                                                       \
> -        if (buf[j].bits > 3*nb_bits || buf[j].bits>32) {                    \
> -            av_log(NULL, AV_LOG_ERROR, "Too long VLC (%d) in init_vlc\n", buf[j].bits);\
> +        if (len > 3*nb_bits || len > 32) {                                  \
> +            av_log(NULL, AV_LOG_ERROR, "Too long VLC (%u) in init_vlc\n", len);\
>              if (buf != localbuf)                                            \
>                  av_free(buf);                                               \
>              return AVERROR(EINVAL);                                         \
>          }                                                                   \
> +        buf[j].bits = len;                                                  \
>          GET_DATA(buf[j].code, codes, i, codes_wrap, codes_size);            \
>          if (buf[j].code >= (1LL<<buf[j].bits)) {                            \
>              av_log(NULL, AV_LOG_ERROR, "Invalid code %"PRIx32" for %d in "  \
> @@ -329,10 +331,10 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int nb_codes,
>              buf[j].symbol = i;                                              \
>          j++;                                                                \
>      }
> -    COPY(buf[j].bits > nb_bits);
> +    COPY(len > nb_bits);
>      // qsort is the slowest part of init_vlc, and could probably be improved or avoided
>      AV_QSORT(buf, j, struct VLCcode, compare_vlcspec);
> -    COPY(buf[j].bits && buf[j].bits <= nb_bits);
> +    COPY(len && len <= nb_bits);
>      nb_codes = j;
>  
>      ret = build_table(vlc, nb_bits, nb_codes, buf, flags);
> 
Will apply this later today unless there are objections.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c
index 77c2b9ce05..ffa352b28b 100644
--- a/libavcodec/bitstream.c
+++ b/libavcodec/bitstream.c
@@ -302,15 +302,17 @@  int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int nb_codes,
     j = 0;
 #define COPY(condition)\
     for (i = 0; i < nb_codes; i++) {                                        \
-        GET_DATA(buf[j].bits, bits, i, bits_wrap, bits_size);               \
+        unsigned len;                                                       \
+        GET_DATA(len, bits, i, bits_wrap, bits_size);                       \
         if (!(condition))                                                   \
             continue;                                                       \
-        if (buf[j].bits > 3*nb_bits || buf[j].bits>32) {                    \
-            av_log(NULL, AV_LOG_ERROR, "Too long VLC (%d) in init_vlc\n", buf[j].bits);\
+        if (len > 3*nb_bits || len > 32) {                                  \
+            av_log(NULL, AV_LOG_ERROR, "Too long VLC (%u) in init_vlc\n", len);\
             if (buf != localbuf)                                            \
                 av_free(buf);                                               \
             return AVERROR(EINVAL);                                         \
         }                                                                   \
+        buf[j].bits = len;                                                  \
         GET_DATA(buf[j].code, codes, i, codes_wrap, codes_size);            \
         if (buf[j].code >= (1LL<<buf[j].bits)) {                            \
             av_log(NULL, AV_LOG_ERROR, "Invalid code %"PRIx32" for %d in "  \
@@ -329,10 +331,10 @@  int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int nb_codes,
             buf[j].symbol = i;                                              \
         j++;                                                                \
     }
-    COPY(buf[j].bits > nb_bits);
+    COPY(len > nb_bits);
     // qsort is the slowest part of init_vlc, and could probably be improved or avoided
     AV_QSORT(buf, j, struct VLCcode, compare_vlcspec);
-    COPY(buf[j].bits && buf[j].bits <= nb_bits);
+    COPY(len && len <= nb_bits);
     nb_codes = j;
 
     ret = build_table(vlc, nb_bits, nb_codes, buf, flags);