diff mbox

[FFmpeg-devel] pixdesc: Only check against valid entries when iterating over lists of enums

Message ID 20180608154705.144075-1-derek.buitenhuis@gmail.com
State Superseded
Headers show

Commit Message

Derek Buitenhuis June 8, 2018, 3:47 p.m. UTC
Some of these enums have gaps in between their values, since they correspond
to the values in various specs, instead of being an incrementing list.

Fixes segfaults when, for example, using the valid API call:

   av_color_primaries_from_name("jecdec-p22");

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavutil/pixdesc.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

Comments

Michael Niedermayer June 9, 2018, 5:31 p.m. UTC | #1
On Fri, Jun 08, 2018 at 04:47:05PM +0100, Derek Buitenhuis wrote:
> Some of these enums have gaps in between their values, since they correspond
> to the values in various specs, instead of being an incrementing list.
> 
> Fixes segfaults when, for example, using the valid API call:
> 
>    av_color_primaries_from_name("jecdec-p22");
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
>  libavutil/pixdesc.c | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index ff5c20d50e..f849222aa3 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -2718,7 +2718,12 @@ int av_color_range_from_name(const char *name)
>      int i;
>  
>      for (i = 0; i < FF_ARRAY_ELEMS(color_range_names); i++) {
> -        size_t len = strlen(color_range_names[i]);
> +        size_t len;
> +
> +        if (!color_range_names[i])
> +            continue;
> +
> +        len = strlen(color_range_names[i]);
>          if (!strncmp(color_range_names[i], name, len))
>              return i;
>      }

theres no hole in color_range_names
this may lead to static analyzers complaining about dead code

[...]
Derek Buitenhuis June 10, 2018, 12:27 p.m. UTC | #2
On 09/06/2018 18:31, Michael Niedermayer wrote:
> theres no hole in color_range_names
> this may lead to static analyzers complaining about dead code

v2 sent.

I will send a patch to modify the doxygen for the affected enums as well,
since I think it's likely downstream users may try to iterate over them
in a similar fashion.

- Derek
Derek Buitenhuis June 10, 2018, 12:29 p.m. UTC | #3
On 10/06/2018 13:27, Derek Buitenhuis wrote:
> I will send a patch to modify the doxygen for the affected enums as well,
> since I think it's likely downstream users may try to iterate over them
> in a similar fashion.

Scratch that part, it already is documented, it seems.

- Derek
diff mbox

Patch

diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index ff5c20d50e..f849222aa3 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2718,7 +2718,12 @@  int av_color_range_from_name(const char *name)
     int i;
 
     for (i = 0; i < FF_ARRAY_ELEMS(color_range_names); i++) {
-        size_t len = strlen(color_range_names[i]);
+        size_t len;
+
+        if (!color_range_names[i])
+            continue;
+
+        len = strlen(color_range_names[i]);
         if (!strncmp(color_range_names[i], name, len))
             return i;
     }
@@ -2737,7 +2742,12 @@  int av_color_primaries_from_name(const char *name)
     int i;
 
     for (i = 0; i < FF_ARRAY_ELEMS(color_primaries_names); i++) {
-        size_t len = strlen(color_primaries_names[i]);
+        size_t len;
+
+        if (!color_primaries_names[i])
+            continue;
+
+        len = strlen(color_primaries_names[i]);
         if (!strncmp(color_primaries_names[i], name, len))
             return i;
     }
@@ -2756,7 +2766,12 @@  int av_color_transfer_from_name(const char *name)
     int i;
 
     for (i = 0; i < FF_ARRAY_ELEMS(color_transfer_names); i++) {
-        size_t len = strlen(color_transfer_names[i]);
+        size_t len;
+
+        if (!color_transfer_names[i])
+            continue;
+
+        len = strlen(color_transfer_names[i]);
         if (!strncmp(color_transfer_names[i], name, len))
             return i;
     }
@@ -2775,7 +2790,12 @@  int av_color_space_from_name(const char *name)
     int i;
 
     for (i = 0; i < FF_ARRAY_ELEMS(color_space_names); i++) {
-        size_t len = strlen(color_space_names[i]);
+        size_t len;
+
+        if (!color_space_names[i])
+            continue;
+
+        len = strlen(color_space_names[i]);
         if (!strncmp(color_space_names[i], name, len))
             return i;
     }
@@ -2794,7 +2814,12 @@  int av_chroma_location_from_name(const char *name)
     int i;
 
     for (i = 0; i < FF_ARRAY_ELEMS(chroma_location_names); i++) {
-        size_t len = strlen(chroma_location_names[i]);
+        size_t len;
+
+        if (!chroma_location_names[i])
+            continue;
+
+        len = strlen(chroma_location_names[i]);
         if (!strncmp(chroma_location_names[i], name, len))
             return i;
     }