[LTP] [PATCH] tst_assert: Fix buffer overflow in scanf

[Petr Vorel]

> Hello,

> Cyril Hrubis <chrubis@suse.cz> writes:

> > Hi!
> >> The maximum field width of a string conversion does not include the
> >> null byte. So we can overflow the buffer by one byte.

> >> This can be triggered in ioctl_loop01 with -fsanitize=address even if
> >> the file contents are far less than the buffer size:

> >> tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s
> >> tst_device.c:93: TINFO: Found free device 1 '/dev/loop1'
> >> ioctl_loop01.c:85: TPASS: /sys/block/loop1/loop/partscan = 0
> >> ioctl_loop01.c:86: TPASS: /sys/block/loop1/loop/autoclear = 0
> >> =================================================================
> >> ==293==ERROR: AddressSanitizer: stack-buffer-overflow on address 0xf5c03420 at pc 0xf7952bf8 bp 0xff9cf9f8 sp 0xff9cf5d0
> >> WRITE of size 1025 at 0xf5c03420 thread T0
> >>     #0 0xf7952bf7  (/lib/libasan.so.8+0x89bf7) (BuildId: f8d5331e88e5c1b8a8a55eda0a8e20503ea0d2b9)
> >>     #1 0xf7953879 in __isoc99_vfscanf (/lib/libasan.so.8+0x8a879) (BuildId: f8d5331e88e5c1b8a8a55eda0a8e20503ea0d2b9)
> >>     #2 0x8071f85 in safe_file_scanf /home/rich/qa/ltp/lib/safe_file_ops.c:139
> >>     #3 0x80552ea in tst_assert_str /home/rich/qa/ltp/lib/tst_assert.c:60
> >>     #4 0x804f17a in verify_ioctl_loop /home/rich/qa/ltp/testcases/kernel/syscalls/ioctl/ioctl_loop01.c:87
> >>     #5 0x8061599 in run_tests /home/rich/qa/ltp/lib/tst_test.c:1380
> >>     #6 0x8061599 in testrun /home/rich/qa/ltp/lib/tst_test.c:1463
> >>     #7 0x8061599 in fork_testrun /home/rich/qa/ltp/lib/tst_test.c:1592
> >>     #8 0x806877a in tst_run_tcases /home/rich/qa/ltp/lib/tst_test.c:1686
> >>     #9 0x804e01b in main ../../../../include/tst_test.h:394
> >>     #10 0xf7188294 in __libc_start_call_main (/lib/libc.so.6+0x23294) (BuildId: 87c7a50c8792985dd164f5af2d45b8e91d9f4391)
> >>     #11 0xf7188357 in __libc_start_main@@GLIBC_2.34 (/lib/libc.so.6+0x23357) (BuildId: 87c7a50c8792985dd164f5af2d45b8e91d9f4391)
> >>     #12 0x804e617 in _start ../sysdeps/i386/start.S:111

> >> Address 0xf5c03420 is located in stack of thread T0 at offset 1056 in frame
> >>     #0 0x805525f in tst_assert_str /home/rich/qa/ltp/lib/tst_assert.c:57

> >>   This frame has 1 object(s):
> >>     [32, 1056) 'sys_val' (line 58) <== Memory access at offset 1056 overflows this variable

> > Uff, looking closely at the scanf manual:

> > String input conversions store a terminating null byte ('\0') to mark
> > the end of the input; the maximum field width does not include this
> > terminator.

> > So do I get it right that scanf() actually writes one byte after the
> > size passed after the % character? That sounds a bit evil to me.

> Yes, I suppose the root cause is null terminated strings. ;-)

Interesting.

I dared to merge it (added your Signed-off-by:).
Thanks!

Kind regards,
Petr

> > Anyways:

> > Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

> Thanks