Bug 13875 - client-side crash in IOXIDResolver ResolveOxid out
Summary: client-side crash in IOXIDResolver ResolveOxid out
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: PIDL and libndr (show other bugs)
Version: 4.10.0
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-01 21:47 UTC by Andrew Bartlett
Modified: 2020-04-17 02:15 UTC (History)
2 users (show)

See Also:


Attachments
patch allocating space for empty DUALSTRINGARRAY (1.35 KB, patch)
2019-10-30 02:10 UTC, Douglas Bagnall
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2019-04-01 21:47:39 UTC
Reported by Michael Hanselmann 

echo -n 'c87PMf7CBAUAAAAADgQMBASjfPqKw0KPld6DY87PMfQ=' | \
  base64 -d | bin/ndrdump IOXIDResolver ResolveOxid out
librpc/ndr/ndr_orpc.c:ndr_pull_DUALSTRINGARRAY: num_entries can be 0,
ar->stringbindings and ar->securitybindings allocated as zero-length arrays.

This is in pure PIDL-generated code so the same issue may appear elsewhere
Comment 1 hansmi 2019-04-03 21:00:22 UTC
More test cases:

echo -n 'AAAAAQ0K9Q0AAAAAAAAAA6ampqampqampqampqampqampqampqamNAAAAAAtNDQ=' | \
  base64 -d | bin/ndrdump IOXIDResolver ResolveOxid2 out

echo -n 'AAAAAQ02CgoKCgoAAAAAAAAAAwAAAAEAADM5NjE2MTc3Njg0MjT8haxJC2GHCgoK9QA=' | \
  base64 -d | bin/ndrdump IOXIDResolver ServerAlive2 out

echo -n 'AAAAAQAAAAAAAABKAAD/AAAAAP4AAAAAAAAASgAAAAAAAAABIiIjIiIiIiIiIiIiIiMiAAAAAAD/AAAAAAAA' | \
  base64 -d | bin/ndrdump IRemoteActivation RemoteActivation out

When built from master branch at 377d27359ccdb8f2680fda36ca388f44456590e5 the last case gives:

==17632==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x608000000300 at pc 0x7f8faebb9717 bp 0x7ffe934d74f0 sp 0x7ffe934d74e8
WRITE of size 8 at 0x608000000300 thread T0
    #0 0x7f8faebb9716 in ndr_pull_DUALSTRINGARRAY /src/samba-upstream/bin/default/../../librpc/ndr/ndr_orpc.c:41:24
    #1 0x7f8fb38a36bd in ndr_pull_RemoteActivation /src/samba-upstream/bin/default/librpc/gen_ndr/ndr_remact.c:285:4
    #2 0x55eb4eef67c0 in main /src/samba-upstream/bin/default/../../librpc/tools/ndrdump.c:456:12
    #3 0x7f8fadbc509a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #4 0x55eb4ee1de79 in _start (/src/samba-upstream/bin/default/librpc/tools/ndrdump+0x4de79)

0x608000000300 is located 0 bytes to the right of 96-byte region [0x6080000002a0,0x608000000300)
allocated by thread T0 here:
    #0 0x55eb4eec5cc3 in __interceptor_malloc (/src/samba-upstream/bin/default/librpc/tools/ndrdump+0xf5cc3)
    #1 0x7f8fb4178175 in __talloc_with_prefix /src/samba-upstream/bin/default/../../lib/talloc/talloc.c:782:9
    #2 0x7f8fb415ebb5 in _talloc_named_const /src/samba-upstream/bin/default/../../lib/talloc/talloc.c:981:8
    #3 0x7f8fb41761e5 in _talloc_array /src/samba-upstream/bin/default/../../lib/talloc/talloc.c:2764:9
    #4 0x7f8faebb80b2 in ndr_pull_DUALSTRINGARRAY /src/samba-upstream/bin/default/../../librpc/ndr/ndr_orpc.c:40:23
    #5 0x7f8fb38a36bd in ndr_pull_RemoteActivation /src/samba-upstream/bin/default/librpc/gen_ndr/ndr_remact.c:285:4
    #6 0x55eb4eef67c0 in main /src/samba-upstream/bin/default/../../librpc/tools/ndrdump.c:456:12
    #7 0x7f8fadbc509a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/samba-upstream/bin/default/../../librpc/ndr/ndr_orpc.c:41:24 in ndr_pull_DUALSTRINGARRAY
Comment 2 Douglas Bagnall 2019-10-30 02:10:11 UTC
Created attachment 15578 [details]
patch allocating space for empty DUALSTRINGARRAY

This patch fixes the immediate problem.

The ndrdump call now crashes in the print routines.
Comment 3 Douglas Bagnall 2019-10-30 03:12:09 UTC
(In reply to Douglas Bagnall from comment #2)
> This patch fixes the immediate problem

I had only tried the first one, but it does seem to have fixed them all (where "fixed" means crashing in print, not pull)
Comment 4 Douglas Bagnall 2019-11-08 04:50:46 UTC
This is in handwritten code, so is not a general problem with NDR code. DCOM is not used anywhere.
Comment 5 Andrew Bartlett 2019-11-18 20:34:56 UTC
Michael Hanselmann,

Can you confirm you are happy to be credited by name in the commit message for this, and let me know any details you want included (eg the name of the fuzzing tool you used)?

Thanks!

Andrew Bartlett
Comment 6 hansmi 2019-11-18 22:22:32 UTC
(In reply to Andrew Bartlett from comment #5)
Yes, I'm happy to be credited. The primary tool was Honggfuzz in combination with fuzzing code which is not yet, as of this writing, available in a public repository. I intend to take care of that soon.
Comment 7 hansmi 2019-11-19 00:21:14 UTC
(In reply to Douglas Bagnall from comment #2)
I re-ran my fuzzing code after applying your patch on top of master commit d6fbfb276ce and there have been no crashes anymore. Thanks a lot!
Comment 8 Douglas Bagnall 2020-04-17 02:15:51 UTC
fixed in 536a84935ce7647f43528d6d376f6762c5e8eb78