bug#72245: [PATCH] Fix integer overflow when reading XPM

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 22 Jul 2024 07:35:55 -0700
> 
> Since XPM files are untrusted input, I think we'd better handle integer
> overflow when parsing it, in case the file is malformed.
> 
> Proposed patch attached.

Thanks.

Paul, any comments or suggestions?

> From 2aa0e1ac9705201939b30a8ca39b3354cbd62a8e Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 22 Jul 2024 16:00:30 +0200
> Subject: [PATCH] Fix integer overflow when reading XPM
> 
> * src/image.c (xpm_str_to_int): New function.
> (xpm_load_image): Avoid integer overflow when reading XPM by replacing
> sscanf with strtol, to correctly handle integer overflow when reading a
> malformed XPM file.
> ---
>  src/image.c | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/src/image.c b/src/image.c
> index 90e6312e128..d8a8dc57ea9 100644
> --- a/src/image.c
> +++ b/src/image.c
> @@ -19,6 +19,7 @@ Copyright (C) 1989-2024 Free Software Foundation, Inc.
>  
>  #include <config.h>
>  
> +#include <errno.h>
>  #include <fcntl.h>
>  #include <math.h>
>  #include <unistd.h>
> @@ -6254,6 +6255,27 @@ xpm_str_to_color_key (const char *s)
>    return -1;
>  }
>  
> +static int
> +xpm_str_to_int (char **buf)
> +{
> +  char *p;
> +
> +  errno = 0;
> +  long result = strtol (*buf, &p, 10);
> +  if (p == *buf || errno == ERANGE || errno == EINVAL
> +      || result < INT_MIN || result > INT_MAX)
> +    return -1;
> +
> +  /* Error out if we see something like "12x3xyz".  */
> +  if (!c_isspace (*p) && *p != '\0')
> +    return -1;
> +
> +  /* Update position to read next integer.  */
> +  *buf = p;
> +
> +  return (int)result;
> +}
> +
>  static bool
>  xpm_load_image (struct frame *f,
>                  struct image *img,
> @@ -6311,10 +6333,14 @@ #define expect_ident(IDENT)                           
>         \
>      goto failure;
>    memcpy (buffer, beg, len);
>    buffer[len] = '\0';
> -  if (sscanf (buffer, "%d %d %d %d", &width, &height,
> -           &num_colors, &chars_per_pixel) != 4
> -      || width <= 0 || height <= 0
> -      || num_colors <= 0 || chars_per_pixel <= 0)
> +  char *next_int = buffer;
> +  if ((width = xpm_str_to_int (&next_int)) <= 0)
> +    goto failure;
> +  if ((height = xpm_str_to_int (&next_int)) <= 0)
> +    goto failure;
> +  if ((num_colors = xpm_str_to_int (&next_int)) <= 0)
> +    goto failure;
> +  if ((chars_per_pixel = xpm_str_to_int (&next_int)) <= 0)
>      goto failure;
>  
>    if (!check_image_size (f, width, height))
> -- 
> 2.45.2
>