* [LTP] [PATCH] tst_assert: Fix buffer overflow in scanf
@ 2023-01-20 13:56 Richard Palethorpe via ltp
2023-01-20 15:05 ` Cyril Hrubis
0 siblings, 1 reply; 4+ messages in thread
From: Richard Palethorpe via ltp @ 2023-01-20 13:56 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
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
Fixes: f4919b145a9e lib: Add TST_ASSERT_FILE_INT and TST_ASSERT_FILE_STR
---
lib/tst_assert.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/tst_assert.c b/lib/tst_assert.c
index 9b8ebc167..b68bd5d39 100644
--- a/lib/tst_assert.c
+++ b/lib/tst_assert.c
@@ -57,7 +57,7 @@ void tst_assert_str(const char *file, const int lineno, const char *path, const
{
char sys_val[1024];
- safe_file_scanf(file, lineno, NULL, path, "%1024s", sys_val);
+ safe_file_scanf(file, lineno, NULL, path, "%1023s", sys_val);
if (!strcmp(val, sys_val)) {
tst_res_(file, lineno, TPASS, "%s = '%s'", path, val);
return;
@@ -71,7 +71,7 @@ void tst_assert_file_str(const char *file, const int lineno, const char *path, c
char sys_val[1024];
char fmt[2048];
- snprintf(fmt, sizeof(fmt), "%s: %%1024s", prefix);
+ snprintf(fmt, sizeof(fmt), "%s: %%1023s", prefix);
file_lines_scanf(file, lineno, NULL, 1, path, fmt, sys_val);
if (!strcmp(val, sys_val)) {
--
2.39.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [LTP] [PATCH] tst_assert: Fix buffer overflow in scanf
2023-01-20 13:56 [LTP] [PATCH] tst_assert: Fix buffer overflow in scanf Richard Palethorpe via ltp
@ 2023-01-20 15:05 ` Cyril Hrubis
2023-01-20 15:11 ` Richard Palethorpe
0 siblings, 1 reply; 4+ messages in thread
From: Cyril Hrubis @ 2023-01-20 15:05 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: ltp
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.
Anyways:
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [LTP] [PATCH] tst_assert: Fix buffer overflow in scanf
2023-01-20 15:05 ` Cyril Hrubis
@ 2023-01-20 15:11 ` Richard Palethorpe
2023-01-25 21:38 ` Petr Vorel
0 siblings, 1 reply; 4+ messages in thread
From: Richard Palethorpe @ 2023-01-20 15:11 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
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. ;-)
>
> Anyways:
>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Thanks
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [LTP] [PATCH] tst_assert: Fix buffer overflow in scanf
2023-01-20 15:11 ` Richard Palethorpe
@ 2023-01-25 21:38 ` Petr Vorel
0 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2023-01-25 21:38 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: ltp
> 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
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-25 21:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-20 13:56 [LTP] [PATCH] tst_assert: Fix buffer overflow in scanf Richard Palethorpe via ltp
2023-01-20 15:05 ` Cyril Hrubis
2023-01-20 15:11 ` Richard Palethorpe
2023-01-25 21:38 ` Petr Vorel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox