Re: possible overrun in wxt_gui.c

[Ethan A M. <me...@uw...>]

On Monday, 2 December 2024 03:43:01 PST Jun T wrote:
> When compiling wxt_gui.cpp, clang (on my Mac) gives warnings,
> in function wxt_update_mousecoords_in_window(), that strongly
> recommend using nsprintf() instead of sprintf().

Applied. Thanks

> 
> While trying to silence this warning, I've found that there
> is a possibility of overrun.
> 
> The format "%10g" (equivalent to "%10.6g") may use up to 13
> characters: "-1.23456e+123". So the buffer mouse_format[]
> may be overrun by the last sprintf(m, "y2=...",...).
> 
> In the patch below I removed some extra spaces in the format
> so that the string would look like (but not exactly the same
> as) the one in the status bar of the active window.
> 
> The condition 'left > 0' is added just in case "%10g" occupies
> more than 13 characters (I guess this should never happen).
> 
> 
> diff --git a/src/wxterminal/wxt_gui.cpp b/src/wxterminal/wxt_gui.cpp
> index 2cd096f01..d5249df58 100644
> --- a/src/wxterminal/wxt_gui.cpp
> +++ b/src/wxterminal/wxt_gui.cpp
> @@ -129,6 +129,9 @@ extern "C" {
>  #ifdef HAVE_SYS_SELECT_H
>  # include <sys/select.h>
>  #endif
> +#ifdef HAVE_WORKING_FORK
> +# include <pango/pangocairo.h>	// for pango_cairo_font_map_set_default()
> +#endif
>  }
>  
>  /* Interactive toggle control variables
> @@ -3272,29 +3275,17 @@ static void wxt_update_mousecoords_in_window(int number, int mx, int my)
>  	if ((window = wxt_findwindowbyid(number))) {
>  
>  		/* TODO: rescale mx and my using stored per-plot scale info */
> -		char mouse_format[66];
> +		char mouse_format[67]; // %10g may use at most 13 chars
>  		char *m = mouse_format;
> -		double x, y, x2, y2;
> -
> -		if (window->axis_mask & (1<<0)) {
> -			x = mouse_to_axis(mx, &window->axis_state[0]);
> -			sprintf(m, "x=  %10g   %c", x, '\0');
> -			m += 17;
> -		}
> -		if (window->axis_mask & (1<<1)) {
> -			y = mouse_to_axis(my, &window->axis_state[1]);
> -			sprintf(m, "y=  %10g   %c", y, '\0');
> -			m += 17;
> -		}
> -		if (window->axis_mask & (1<<2)) {
> -			x2 = mouse_to_axis(mx, &window->axis_state[2]);
> -			sprintf(m, "x2=  %10g   %c", x2, '\0');
> -			m += 17;
> -		}
> -		if (window->axis_mask & (1<<3)) {
> -			y2 = mouse_to_axis(my, &window->axis_state[3]);
> -			sprintf(m, "y2=  %10g %c", y2, '\0');
> -			m += 15;
> +		int left = sizeof(mouse_format);
> +		const char *name[4] = { "x", "y", "x2", "y2" };
> +		for (int i=0; i<4 && left > 0; ++i) {
> +			if (window->axis_mask & (1<<i)) {
> +				int n = snprintf(m, left, "%s=% -10g ", name[i],
> +							mouse_to_axis(mx, &window->axis_state[i]));
> +				m += n;
> +				left -= n;	// left<=0 if truncated (should not happen)
> +			}
>  		}
>  
>  		FPRINTF((stderr,"wxt : update mouse coords in window %d\n",number));