[PATCH] getty: Fix potential out-of-range error in parse_speeds()

[Harald van Dijk]

On 23/08/2024 13:00, Maks Mishin wrote:
> Accessing an element of array 'G->speeds' of size 10
> at getty.c:165 can lead to a buffer overflow, since the index
> 'G->numspeed' can have an out of range value 10,
> as indicated by a preceding conditional expression at getty.c:170.
> 
> Found by the static analyzer Svace.

The static analyzer found a legitimate bug, but the fix does not look 
correct to me. It is not an error for G.numspeed to equal 10. It is only 
an error for G.numspeed to equal 10 and then another speed being added.

> Signed-off-by: Maks Mishin <maks.mishinFZ at gmail.com>
> ---
>   loginutils/getty.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/loginutils/getty.c b/loginutils/getty.c
> index 4581cc9f7..0542cf83a 100644
> --- a/loginutils/getty.c
> +++ b/loginutils/getty.c
> @@ -167,7 +167,7 @@ static void parse_speeds(char *arg)
>   			bb_error_msg_and_die("bad speed: %s", cp);
>   		/* note: arg "0" turns into speed B0 */
>   		G.numspeed++;
> -		if (G.numspeed > MAX_SPEED)
> +		if (G.numspeed >= MAX_SPEED)
>   			bb_simple_error_msg_and_die("too many alternate speeds");
>   	}
>   	debug("exiting parse_speeds\n");

Does it work to check G.numspeed >= MAX_SPEED at the start of the loop, 
prior to G.numspeed being incremented?

Cheers,
Harald van Dijk