linux-kernel - Re: [PATCH v2] speakup: keyhelp: guard letter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aNrHxtOpz9wxnY_0@begin>
Date: Mon, 29 Sep 2025 19:54:14 +0200
From: Samuel Thibault <samuel.thibault@...-lyon.org>
To: Pavel Zhigulin <Pavel.Zhigulin@...persky.com>
Cc: William Hubbs <w.d.hubbs@...il.com>,
	Chris Brannon <chris@...-brannons.com>,
	Kirk Reiser <kirk@...sers.ca>, speakup@...ux-speakup.org,
	linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH v2] speakup: keyhelp: guard letter_offsets possible
 out-of-range indexing

Pavel Zhigulin, le lun. 29 sept. 2025 14:03:45 +0300, a ecrit:
> help_init() builds letter_offsets[] by using the first byte of each
> function name as an index via `(start & 31) - 1`. If function_names are
> overridden from sysfs (root) with a name starting outside [a–z], the
> index underflows or exceeds the array, leading to OOB write.
> 
> Function names can be overridden with the following commands as root:
> 
>     modprobe speakup_soft
>     echo "0 _bad" > /sys/accessibility/speakup/i18n/function_names
>     # then press Insert+2 on /dev/tty
> 
> This fix checks the first letter in help_init(), and if it is not in the
> [a–z] range the function returns an error to the caller. Eventually this
> error is propagated to drivers/accessibility/speakup/main.c:2217, which
> causes a bleep sound.
> 
> Fixes: c6e3fd22cd53 ("Staging: add speakup to the staging directory")
> Signed-off-by: Pavel Zhigulin <Pavel.Zhigulin@...persky.com>

Reviewed-by: Samuel Thibault <samuel.thibault@...-lyon.org>

Thanks!


> ---
> v2: Use a proper commit in the 'Fixes' trailer. Remove the redundant
> NULL check in help_init() and make it return void as
> Samuel Thibault <samuel.thibault@...-lyon.org> suggested during
> review.
> 
>  drivers/accessibility/speakup/keyhelp.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/accessibility/speakup/keyhelp.c b/drivers/accessibility/speakup/keyhelp.c
> index 822ceac83068..e632c53d6246 100644
> --- a/drivers/accessibility/speakup/keyhelp.c
> +++ b/drivers/accessibility/speakup/keyhelp.c
> @@ -8,6 +8,7 @@
>   */
> 
>  #include <linux/keyboard.h>
> +#include <linux/ctype.h>
>  #include "spk_priv.h"
>  #include "speakup.h"
> 
> @@ -111,7 +112,7 @@ static void say_key(int key)
>  			     spk_msg_get(MSG_KEYNAMES_START + (key - 1)));
>  }
> 
> -static int help_init(void)
> +static void help_init(void)
>  {
>  	char start = SPACE;
>  	int i;
> @@ -120,13 +121,19 @@ static int help_init(void)
>  	state_tbl = spk_our_keys[0] + SHIFT_TBL_SIZE + 2;
>  	for (i = 0; i < num_funcs; i++) {
>  		char *cur_funcname = spk_msg_get(MSG_FUNCNAMES_START + i);
> +		char first_letter;
> 
> -		if (start == *cur_funcname)
> +		first_letter = tolower(*cur_funcname);
> +
> +		/* Accept only 'a'..'z' to index letter_offsets[] safely */
> +		if (first_letter < 'a' || first_letter > 'z')
> +			continue;
> +
> +		if (start == first_letter)
>  			continue;
> -		start = *cur_funcname;
> +		start = first_letter;
>  		letter_offsets[(start & 31) - 1] = i;
>  	}
> -	return 0;
>  }
> 
>  int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
> @@ -144,7 +151,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
>  			synth_printf("%s\n", spk_msg_get(MSG_LEAVING_HELP));
>  			return 1;
>  		}
> -		ch |= 32; /* lower case */
> +		ch = tolower(ch);
>  		if (ch < 'a' || ch > 'z')
>  			return -1;
>  		if (letter_offsets[ch - 'a'] == -1) {
> --
> 2.43.0
> 

-- 
Samuel
 RM> Mauvais OS, changer d'OS (c)(r)(tm)
 J'ai windows 98 et comment faire pour changer l'os de windows 98?
 Dans ajout et suppression du programme et il ne parle pas d'os.
 -+- DN in : GNU -+- L'O.S. est las, hélas, c'est là qu'est l'os -+-