Tcl Source Code

View Ticket
Login
Ticket UUID: ab123cfd3d02fb1198f18261c9705e09b4797b99
Title: ValidateFormat(): signed integer overflow
Type: Bug Version: core-8-6-branch
Submitter: chrstphrchvz Created on: 2023-04-27 04:12:21
Subsystem: 18. Commands M-Z Assigned To: nobody
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2023-05-02 18:22:18
Resolution: Fixed Closed By: chrstphrchvz
    Closed on: 2023-05-02 18:22:18
Description:

Triggered by scan-13.9 on 9.0. -fsanitize=signed-integer-overflow error for 32-bit long:

generic/tclScan.c:321:15: runtime error: signed integer overflow: -2147483648 - 1 cannot be represented in type 'long int'

User Comments: chrstphrchvz added on 2023-05-02 18:22:18:

Be aware that UBSan says to avoid -lubsan; using gcc or clang as the linker with -fsanitize=undefned should be enough.


apnadkarni added on 2023-05-02 17:59:22:
Fixed in all three branches. Serendipity that one of the other bugs I was looking at affected 8.6 as well.

Tested with `../configure CFLAGS="-m32 -fsanitize=signed-integer-overflow" LDFLAGS="-lubsan"` (and patched dltest Makefile) but please test and re-open if issue persists.

Thanks for the patch.

apnadkarni added on 2023-05-02 11:04:46:
(Not saying it's unimportant but my focus is on other areas for the moment)

apnadkarni added on 2023-05-02 11:02:56:
Fixed the commit message. As for 8.6, 8.7, that would have to wait I'm afraid just as a matter of priority.

chrstphrchvz added on 2023-05-02 10:39:56:

Reopening because I would still like for this to be fixed in 8.6 and 8.7.

See attached patch for core-8-6-branch. (For core-8-branch, [3da8d2069c69] can be applied directly along with copying over scan-13.9. But for 8.6, I am not sure anyone would want to bring back compat/strtoull.c, which was removed in 2007.) I agree that storing the result of strtoul() in a signed int caused several issues, including large arguments being erroneously accepted due to truncation from 64-bit to 32-bit, the comment about value/longval being “guaranteed to be > 0” not being true, and the signed integer overflow and the scan-13.9 crash.

To clarify, the overflow is detected by UBSan (UndefinedBehaviorSanitizer) rather than ASan (AddressSanitizer); consider revising the check-in message for [787712b4c733] and [3da8d2069c69].

ASan, like Valgrind, is really only useful with PURIFY defined. I usually configure with both UBSan and ASan enabled, using CFLAGS='-DPURIFY -fsanitize=address -fsanitize=undefined' LDFLAGS='-fsanitize=address' ./configure --enable-symbols … ; I haven’t had to specify -fsanitize=undefined in LDFLAGS so far. I also apply a workaround for [ac874937c5]. I then usually run with ASAN_OPTIONS=allocator_may_return_null=1:detect_leaks=0:malloc_context_size=99.


apnadkarni added on 2023-05-01 10:39:11:
Fixed in [3da8d2069c]. Verified on Ubuntu 20.

apnadkarni added on 2023-05-01 06:52:35:
Proposed fix in [bug-ab123cfd3d].

chrstphrchvz added on 2023-04-29 19:21:22:

That error sounds like -fsanitize=undefined might need to be added to LDFLAGS.


apnadkarni added on 2023-04-27 14:29:12:
Fix seems simple but I'd like to reproduce the issue first so I can verify. Could you let me know the configure command for the build? I get a "undefined reference to __ubsan_handle_add_overflow" link error. Sorry, not familiar with address sanitizer.

chrstphrchvz added on 2023-04-27 07:16:01:

This issue as well as [d4ede611a7a7] are present on 8.6 and 8.7, as revealed by running the contents of scan-13.9 (scan abc {%2147483648$s}):

tcl86/generic/tclScan.c:318:15: runtime error: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
tcl86/generic/tclScan.c:495:14: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
tcl86/generic/tclScan.c:684:12: runtime error: signed integer overflow: -2147483648 - 1 cannot be represented in type 'long int'
tcl86/generic/tclScan.c:840:16: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
tcl86/generic/tclScan.c:840:7: runtime error: applying non-zero offset 4294967292 to null pointer
AddressSanitizer:DEADLYSIGNAL
=================================================================
==14995==ERROR: AddressSanitizer: SEGV on unknown address 0x3fffffff (pc 0xb638796b bp 0xbf88eed8 sp 0xbf88ecd0 T0)
==14995==The signal is caused by a READ memory access.
    #0 0xb638796b in Tcl_ScanObjCmd tcl86/generic/tclScan.c:840
    #1 0xb5a80a1c in Dispatch tcl86/generic/tclBasic.c:4467
    #2 0xb5a80e22 in TclNRRunCallbacks tcl86/generic/tclBasic.c:4503
    #3 0xb5a7dbc3 in Tcl_EvalObjv tcl86/generic/tclBasic.c:4226
    #4 0xb5a8c904 in TclEvalEx tcl86/generic/tclBasic.c:5372
    #5 0xb6267bdb in Tcl_FSEvalFileEx tcl86/generic/tclIOUtil.c:1824
    #6 0xb62a34b2 in Tcl_MainEx tcl86/generic/tclMain.c:403
    #7 0x509523 in main tcl86/unix/tclAppInit.c:91
    #8 0xb4a25128  (/usr/lib/libc.so.6+0x1f128)
    #9 0xb4a251fc in __libc_start_main (/usr/lib/libc.so.6+0x1f1fc)
    #10 0x5093ea in _start (tcl86/unix/tcltest+0x443ea)

AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV tcl86/generic/tclScan.c:840 in Tcl_ScanObjCmd ==14995==ABORTING


Attachments: