Fwd: [ ul(1): signed integer overflow in column position tracking ]

[email protected] Wed, 27 May 2026 10:10:52 +0200
Newsgroups gmane.os.openbsd.bugs
Message-ID <[email protected]>
Hello,

Correct mail this time.
Any OK?


----- Forwarded message from Renaud Allard <[email protected]> -----

> From: Renaud Allard <[email protected]>
> To: [email protected]
> Subject: ul(1): signed integer overflow in column position tracking
> Date: Tue, 31 Mar 2026 15:04:23 +0200
> 
> The c_pos and c_width fields in the CHAR structure and the local
> variables w and wt in mfilter() are signed int.  Repeated tab
> characters cause c_pos to accumulate without bound, eventually
> overflowing.
> 
> The tab expansion code at line 202 computes:
> 
>   wt = (obuf[col - 1].c_pos + 8) & ~7;
> 
> and then at line 213:
> 
>   obuf[col++].c_pos = ++w;
> 
> where w starts from the previous c_pos.  With enough tabs
> (especially combined with carriage returns that reset the column
> but not the position), c_pos overflows signed int.
> 
> The same overflow occurs at lines 284, 309, and 314 where c_pos
> is computed as the previous position plus a character width.
> 
> Fix: change c_pos, c_width, w, and wt from int to unsigned int.
> These values are always non-negative and the unsigned arithmetic
> wraps deterministically instead of being undefined behavior.
> 
> Found by AFL++ fuzzing with UBSan.
> 
> Index: usr.bin/ul/ul.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ul/ul.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 ul.c
> --- usr.bin/ul/ul.c	16 Oct 2016 11:28:54 -0000	1.23
> +++ usr.bin/ul/ul.c
> @@ -65,8 +65,8 @@ int	must_use_uc, must_overstrike;
>  struct	CHAR	{
>  	char	c_mode;
>  	wchar_t	c_char;
> -	int	c_width;
> -	int	c_pos;
> +	unsigned int	c_width;
> +	unsigned int	c_pos;
>  } ;
> 
>  struct	CHAR	obuf[MAXBUF];
> @@ -164,7 +164,8 @@ mfilter(FILE *f)
>  {
>  	struct CHAR	*cp;
>  	wint_t		 c;
> -	int		 skip_bs, w, wt;
> +	int		 skip_bs;
> +	unsigned int	 w, wt;
> 
>  	col = 1;
>  	skip_bs = 0;
> 

----- End forwarded message -----