Created attachment 15690 [details] patch for master Original bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19385 ==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6070000000d4 at pc 0x55d680b37109 bp 0x7ffd4b404920 sp 0x7ffd4b4040c8 READ of size 2 at 0x6070000000d4 thread T0 SCARINESS: 14 (2-byte-read-heap-buffer-overflow) #0 0x55d680b37108 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long) /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:839:7 #1 0x55d680b3765a in __interceptor_bcmp /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:885:10 #2 0x55d680d9d771 in ndr_string_length samba/librpc/ndr/ndr_string.c:558:14 #3 0x55d680d9e2fd in ndr_pull_charset_to_null samba/librpc/ndr/ndr_string.c:632:12 #4 0x55d680bd1c07 in ndr_pull_spoolss_DeviceMode samba/bin/default/librpc/gen_ndr/ndr_spoolss.c:1458:3 #5 0x55d680d4b71c in LLVMFuzzerTestOneInput samba/bin/default/lib/fuzzing/fuzz_ndr_spoolss_TYPE_STRUCT.c:265:13 #6 0x55d680ad0d81 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:556:15 #7 0x55d680abb8a1 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:292:6 #8 0x55d680ac155e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:774:9 #9 0x55d680aeb482 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:19:10 #10 0x7f265acb682f in __libc_start_main #11 0x55d680a94ce8 in _start
I've worked on this bug with Günther. The ndr_string_length() function has no input length and if the string doesn't have a null terminator, we will continue reading the the memory till we hopefully find one.
This is bad, but I'm not sure if this has any security relevance.
This adds a deprecated flag but does not remove the references from pidl/lib/Parse/Pidl/Samba4/NDR/Client.pm and pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm Those are in the push/print case, the bug here is that ndr_string_length is being used on untrusted input in the pull case. We are already breaking the ABI for ndr-1.0.0, so perhaps just rename ndr_string_length() -> ndr_trusted_string_length() and add ndr_string_n_length() with the extra buffer size argument? Finally, in terms of paying a security bug, do we care if a user can crash the spoolss deamon if configured that way? Otherwise it would 'just' be an smbd self-DoS, which we don't pay on. We have may security bugs, I don't think we need the extra work for this one. Some of our distributors (specifically Debian) are not universally shipping our security patches as is, so perhaps our bar is too low. For contrast, see here for a little guidance on a previous spoolss NDR issue (before we realised it couldn't be exploited): https://bugzilla.samba.org/show_bug.cgi?id=13876#c16
Created attachment 15717 [details] patch for 4.11
Created attachment 15718 [details] patch for 4.10
Reassigning to Karolin for inclusion in 4.10 and 4.11.
The patch is in master, removing embargo. No CVE.
(In reply to Ralph Böhme from comment #6) Pushed to autobuild-v4-{11,10}-test.
(In reply to Karolin Seeger from comment #8) Pushed to both branches. Closing out bug report. Thanks!