Detailed Report: https://oss-fuzz.com/testcase?key=5705660323069952
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
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:
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 proposed patch, need to add tests for the new macros and reword the commits.
Created attachment 15777 [details]
Proposed patch for master
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.