Bug 12746 - possible negative array access in debug_backends_log
Summary: possible negative array access in debug_backends_log
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on:
Reported: 2017-04-15 20:27 UTC by Hanno Böck
Modified: 2017-04-21 07:01 UTC (History)
3 users (show)

See Also:

patch to avoid negative array access (419 bytes, patch)
2017-04-15 20:27 UTC, Hanno Böck
no flags Details
git-am fix for 4.6.next, 4.5.next. (1.10 KB, patch)
2017-04-19 00:32 UTC, Jeremy Allison
asn: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Hanno Böck 2017-04-15 20:27:52 UTC
Created attachment 13154 [details]
patch to avoid negative array access

The function debug_backends_log in lib/util/debug.c contains this code:

if (msg[len - 1] == '\n') {

len can be zero and thus this can lead to a -1 array access reading invalid memory.

The fix is to have a check for len > = before, like this:
if ((len > 0) && (msg[len - 1] == '\n')) {

Patch attached. (Patch is against 4.6.2, but also applies to git head)

This can be detected with address sanitizer, here's the stack trace from it:
==26759==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f1ca6fc949f at pc 0x7f1ca6dc2eb8 bp 0x7ffcf1af7430 sp 0x7ffcf1af7428
READ of size 1 at 0x7f1ca6fc949f thread T0
    #0 0x7f1ca6dc2eb7 in debug_backends_log ../lib/util/debug.c:400
    #1 0x7f1ca6dc30bd in Debug1 ../lib/util/debug.c:1129
    #2 0x7f1ca6dc3ed2 in bufr_print ../lib/util/debug.c:1145
    #3 0x7f1ca6dc3ed2 in dbgflush ../lib/util/debug.c:1209
    #4 0x7f1ca4ecf4cd in dump_core ../source3/lib/dumpcore.c:323
    #5 0x7f1ca4eaeabc in smb_panic_s3 ../source3/lib/util.c:814
    #6 0x7f1ca7bd6a07 in smb_panic ../lib/util/fault.c:166
    #7 0x7f1ca7bd6e1b in fault_report ../lib/util/fault.c:83
    #8 0x7f1ca7bd6e1b in sig_fault ../lib/util/fault.c:94
    #9 0x7f1ca8055f3f  (/lib64/libpthread.so.0+0x10f3f)
    #10 0x7f1ca2fab02e in raise (/lib64/libc.so.6+0x3302e)
    #11 0x7f1ca2fac429 in abort (/lib64/libc.so.6+0x34429)
    #12 0x7f1ca833cb08  (/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/libasan.so.3+0xdbb08)
    #13 0x7f1ca83321aa  (/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/libasan.so.3+0xd11aa)
    #14 0x7f1ca832c2f6  (/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/libasan.so.3+0xcb2f6)
    #15 0x7f1ca82af9be in getgroups (/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/libasan.so.3+0x4e9be)
    #16 0x7f1ca7371143 in get_current_groups ../source3/smbd/sec_ctx.c:156
    #17 0x7f1ca7371143 in init_sec_ctx ../source3/smbd/sec_ctx.c:466
    #18 0x55e7f825c06d in main ../source3/smbd/server.c:1667
    #19 0x7f1ca2f981e0 in __libc_start_main (/lib64/libc.so.6+0x201e0)
    #20 0x55e7f825e049 in _start (/usr/sbin/smbd+0x13049)

0x7f1ca6fc949f is located 1 bytes to the left of global variable 'format_bufr' defined in '../lib/util/debug.c:486:13' (0x7f1ca6fc94a0) of size 1024
0x7f1ca6fc949f is located 55 bytes to the right of global variable 'format_pos' defined in '../lib/util/debug.c:487:19' (0x7f1ca6fc9460) of size 8
SUMMARY: AddressSanitizer: global-buffer-overflow ../lib/util/debug.c:400 in debug_backends_log
Comment 1 Jeremy Allison 2017-04-19 00:32:41 UTC
Created attachment 13159 [details]
git-am fix for 4.6.next, 4.5.next.

Cherry-picked from master.
Comment 2 Andreas Schneider 2017-04-19 06:02:47 UTC
Karolin, please add the patch to the relevant branches. Thanks.
Comment 3 Karolin Seeger 2017-04-19 09:35:23 UTC
(In reply to Andreas Schneider from comment #2)
Pushed to autobuild-v4-{5,6}-test.
Comment 4 Karolin Seeger 2017-04-21 07:01:21 UTC
(In reply to Karolin Seeger from comment #3)
Pushed to both branches.
Closing out bug report.