smbutil view crashes in ndr_clnt_get_frags()
In testing #13169 (and re-tested against illumos-gate without that change), I discovered that, when the NDR client attempts to reassemble fragmented responses, it cores due to tripping the stack smash detection in ndr_clnt_get_frags():
Loading modules: [ libc.so.1 libuutil.so.1 libavl.so.1 ld.so.1 ] > $c libc.so.1`syscall+0x13(fef0c3c4, 1c, 58dcef98, 0, 8047a28, fedee000) 0xfee8d48f(8047ab4) libmlrpc.so.2`__stack_chk_fail_local+0x17(fedd6ab1, 8047910, 10000, 806c498, 8047a5c, 200f000) libmlrpc.so.2`ndr_clnt_get_frags+0x79(806c428, 8047ab0) libmlrpc.so.2`ndr_clnt_call+0x2dd(806c454, f, 8047c7c) libmlrpc.so.2`ndr_rpc_call+0x38(8047ce4, f, 8047c7c) srvsvc_net_share_enum+0x98(8047ce4, 8069668, 1, 8047ce0, fedca000, 806c428) share_enum_rpc+0xa4(8069c50, 8069668, 3, fedca000, 0, 8067420) cmd_view+0x191(2, 8047e04, 8047d98, 80533c1) main+0xc1(8047d9c, fef6d628, 8047dd8, 8053017, 3, 8047e00) _start_crt+0x96(3, 8047e00, fefd06ee, 0, 0, 0) _start+0x1a(3, 8047ed0, 8047ed8, 8047edd, 0, 8047ef5)
That function has a local variable, ndr_common_header_t hdr, which gets passed to ndr_clnt_get_frag, which passes it to ndr_decode_frag_hdr. That function then copies 24 bytes of data (the size of an NDR response header) from the message directly into the structure. However, ndr_common_header_t is only 16-bytes long, and so it writes 8 bytes too many to the stack.
After the recent userland stack protector changes, this causes the process to crash in the stack check function. Prior to this change, however, this seemingly has no effect; the extra 8 bytes are written to space on the stack used to align the stack to 16 bytes.
Only the parts of the header in the common header get used for fragment reassembly (in both the client and server), and so the solution is to only copy the common part of the header.
Issue can be reproduced by using 'smbutil view' against a server with a very large number of shares.
Updated by Matt Barden 11 months ago
Testing was done as follows:
1. Configure a server with a very large number of shares (the number of which can be reduced through use of share comments) - enough that the next step spans multiple RPC messages when viewed in wireshark.
2. smbutil view //user@server
If the user is a domain user, and the server is localhost (aka run smbutil from the configured server), this also tests that NTLM pass-thru authentication continues to work.
Updated by Electric Monk 11 months ago
- Status changed from New to Closed
- % Done changed from 0 to 100
commit 4d00d81bf82141f996e032f9c53e0e996a5f7204 Author: Matt Barden <firstname.lastname@example.org> Date: 2021-02-27T16:04:49.000Z 13501 smbutil view crashes in ndr_clnt_get_frags() Reviewed by: Evan Layton <email@example.com> Reviewed by: Gordon Ross <firstname.lastname@example.org> Approved by: Robert Mustacchi <email@example.com>