[PATCH] miscutils: fix DIVISION_BY_ZERO.EX in beep.c

[tito]

On Thu, 30 Jan 2025 14:04:46 +0300
Anton Moryakov <ant.v.moryakov at gmail.com> wrote:

> Report of the static analyzer:
> DIVISION_BY_ZERO.EX Variable xatou(...), 
> whose possible value set allows a zero value at xatonum_template.c:118 by calling function 'xatou' at beep.c:90,
> is used as a denominator at beep.c:90.
> 
> Corrections explained:
> Fixed a potential division by zero issue in beep.c.  
> The function xatou(optarg) could return0, leading to an  undefined behavior when used as a denominator.  
> Changes:  
> - Added a check to ensure the frequency value is nonzero before division.  - If an invalid frequency (0) is provided, the program exits with an error message.  
> This fix prevents crashes and ensures safer execution.
> 
> Triggers found by static analyzer Svace.
> 
> Signed-off-by: Anton Moryakov <ant.v.moryakov at gmail.com>
> 
> ---
>  miscutils/beep.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/miscutils/beep.c b/miscutils/beep.c
> index 724a666c8..1667757ea 100644
> --- a/miscutils/beep.c
> +++ b/miscutils/beep.c
> @@ -87,6 +87,9 @@ int beep_main(int argc, char **argv)
>  		switch (c) {
>  		case 'f':
>  /* TODO: what "-f 0" should do? */
> +			unsigned freq = xatou(optarg);
> +			if (freq == 0) 
> +    			bb_error_msg_and_die("frequency cannot be zero");
>  			tickrate_div_freq = (unsigned)CLOCK_TICK_RATE / xatou(optarg);
>  			continue;
>  		case 'l':

Hi,
wouldn't it be enough  to use xatou_range:

 diff -uNp miscutils/beep.c.orig miscutils/beep.c
--- miscutils/beep.c.orig       2023-07-21 22:23:35.083200110 +0200
+++ miscutils/beep.c    2025-01-30 13:53:32.746176275 +0100
@@ -86,8 +86,7 @@ int beep_main(int argc, char **argv)
  */
                switch (c) {
                case 'f':
-/* TODO: what "-f 0" should do? */
-                       tickrate_div_freq = (unsigned)CLOCK_TICK_RATE / xatou(optarg);
+                       tickrate_div_freq = (unsigned)CLOCK_TICK_RATE / xatou_range(optarg, 1, 20000);
                        continue;
                case 'l':
                        length = xatou(optarg);

Ciao,
Tito

P.S.: even tough man beep says:

-f Beep  with  a tone frequency of FREQ Hz, where 0 < FREQ < 20000
 it also accepts 0 (plays default 440 hz)  and 20000:

 beep -f 20000 --debug --verbose
beep: Verbose: evdev driver_detect 0x55f1d6a00700 (nil)
beep: Verbose: b-lib: opened /dev/input/by-path/platform-pcspkr-event-spkr as 3
beep: Verbose: beep: using driver 0x55f1d6a00700 (name=evdev, fd=3, dev=/dev/input/by-path/platform-pcspkr-event-spkr)
beep: Verbose: 1 times 200 ms beeps (100 ms delay between, 0 ms delay after) @ 20000 Hz
beep: Verbose: evdev driver_begin_tone 0x55f1d6a00700 20000
beep: Verbose: evdev driver_end_tone 0x55f1d6a00700
beep: Verbose: evdev driver_end_tone 0x55f1d6a00700
beep: Verbose: evdev driver_fini 0x55f1d6a00700
root at devuan:/home/tito# beep -f 20001 --debug --verbose

beep -f 0 --debug --verbose
beep: Verbose: evdev driver_detect 0x55ff9aab2700 (nil)
beep: Verbose: b-lib: opened /dev/input/by-path/platform-pcspkr-event-spkr as 3
beep: Verbose: beep: using driver 0x55ff9aab2700 (name=evdev, fd=3, dev=/dev/input/by-path/platform-pcspkr-event-spkr)
beep: Verbose: 1 times 200 ms beeps (100 ms delay between, 0 ms delay after) @ 440 Hz
beep: Verbose: evdev driver_begin_tone 0x55ff9aab2700 440
beep: Verbose: evdev driver_end_tone 0x55ff9aab2700
beep: Verbose: evdev driver_end_tone 0x55ff9aab2700
beep: Verbose: evdev driver_fini 0x55ff9aab2700


but not 20001.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: beep.patch
Type: text/x-patch
Size: 449 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20250130/4561e008/attachment.bin>