Detailed Report: https://oss-fuzz.com/testcase?key=5705660323069952 Project: samba Fuzzing Engine: libFuzzer Fuzz Target: fuzz_ndr_drsuapi_TYPE_OUT Job Type: libfuzzer_asan_samba Platform Id: linux Crash Type: Stack-overflow Crash Address: 0x7ffcb4cc2ff8 Crash State: ndr_pull_drsuapi_DsaAddressListItem_V1 Sanitizer: address (ASAN) Crash Revision: https://oss-fuzz.com/revisions?job=libfuzzer_asan_samba&revision=202001050517 Reproducer Testcase: https://oss-fuzz.com/download?testcase_id=5705660323069952
This is a recursive call to: NDR_CHECK As the code builds up a linked list of drsuapi_DsaAddressListItem_V1 elements. This causes a stack overflow detected by ASAN, although the depth that it triggers at 253 seems a little low.
I've added a depth element to the ndr_push structure, and added a "depth" attribute to the pidl code. - [depth(250)] drsuapi_DsaAddressListItem_V1 *next; If an element has a depth element, the depth value is checked against the attribute value. The depth value is incremented on the call and decremented on the return. A couple of questions. 1) Is this a reasonable approach. 2) I'm not convinced that depth is a good name. 3) What error code should be returned, currently using NDR_ERR_BUFSIZE. Should a new error code be added.
Summary of an off-line conversation with Andrew Bartlet. 1) rename the "depth" attribute "max_recursion" 2) rename the depth "field" on ndr_pull "recursion_depth" 3) add "global_max_recursion" to ndr_pull This will override the value specified for max_recusion 4) modify the fuzzer to set global_max_recursion to 128 to avoid stack overflow when running under ASAN 5) Add NDR_ERR_MAX_RECUSION_EXCEEDED 6) Check the idl for "next" and add max_recursion property 7) If possible, and maybe later. See if pidl can be altered to require max_recursion for recursive structures.
Created attachment 15757 [details] WIP patch WIP proposed patch, need to add tests for the new macros and reword the commits.
Created attachment 15777 [details] Proposed patch for master CI: https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/188868
I've looked over this carefully, and thankfully none of these are parsed server-side. Indeed most are not used, eg the drsblobs.idl structures. The ioctl.idl changes are only used in torture clients. So we can make it public. Gary, can you do a normal MR for these?
fixed in 23d285d34900270fe171f06f3fbad9879004d4a4 The commits refer to oss-fuzz issue 19280, which should be 19820. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19820