Bug 12747 - wrong use of getgroups causes buffer overflow
wrong use of getgroups causes buffer overflow
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-04-15 21:35 UTC by Hanno Böck
Modified: 2017-04-21 07:02 UTC (History)
3 users (show)

See Also:


Attachments
fix use of getgroups (608 bytes, patch)
2017-04-15 21:35 UTC, Hanno Böck
no flags Details
git-am fix for master. (2.73 KB, patch)
2017-04-17 21:46 UTC, Jeremy Allison
no flags Details
git-am fix for 4.6.next, 4.5.next. (3.08 KB, patch)
2017-04-19 00:35 UTC, Jeremy Allison
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hanno Böck 2017-04-15 21:35:53 UTC
Created attachment 13155 [details]
fix use of getgroups

The function gain_root() (source3/smbd/sec_ctx.c) contains this line of code:
ngroups = sys_getgroups(0,&grp);

I'm not sure what was intended here, but it seems to me the function getgroups (for which sys_getgroups is an alias) isn't supposed to be used like this. If I compile samba with address sanitizer it'll report a buffer overflow in grp.

When the first argument is 0 then getgroups isn't supposed to do anything with the second argument, so it can (and should) be a null pointer. This avoids the overflow. grp isn't used anywhere else. Patch attached.

Here's the stack trace from address sanitizer:
==14194==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff3c1a0d84 at pc 0x7feafbed89a1 bp 0x7fff3c1a0d40 sp 0x7fff3c1a04f0
WRITE of size 40 at 0x7fff3c1a0d84 thread T0
    #0 0x7feafbed89a0 in getgroups (/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/libasan.so.3+0x4e9a0)
    #1 0x7feafaf9a0f3 in get_current_groups ../source3/smbd/sec_ctx.c:156
    #2 0x7feafaf9a0f3 in init_sec_ctx ../source3/smbd/sec_ctx.c:466
    #3 0x56489a05e06d in main ../source3/smbd/server.c:1667
    #4 0x7feaf6bc11e0 in __libc_start_main (/lib64/libc.so.6+0x201e0)
    #5 0x56489a060049 in _start (/usr/sbin/smbd+0x13049)

Address 0x7fff3c1a0d84 is located in stack of thread T0 at offset 36 in frame
    #0 0x7feafaf99f5f in init_sec_ctx ../source3/smbd/sec_ctx.c:447

  This frame has 1 object(s):
    [32, 36) 'grp' <== Memory access at offset 36 overflows this variable
Comment 1 Jeremy Allison 2017-04-17 21:46:10 UTC
Created attachment 13156 [details]
git-am fix for master.

I think this is a more complete fix here.
Comment 2 Jeremy Allison 2017-04-19 00:35:01 UTC
Created attachment 13160 [details]
git-am fix for 4.6.next, 4.5.next.

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

Thanks!