Created attachment 18995 [details] original report Tristan (TristanInSec@gmail.com) reports (as attached): `_samr_SetSecurity()` in `source3/rpc_server/samr/srv_samr_nt.c` dereferences `r->in.sdbuf->sd->dacl` at line 726 without checking if `sd` is NULL. The IDL defines `sdbuf` as `[in,ref]` (guaranteed non-NULL by NDR). But `sec_desc_buf.sd` is `[subcontext(4)] security_descriptor *sd`, which is NULL when `sd_size == 0`. dacl = r->in.sdbuf->sd->dacl; // NULL deref: sd is NULL for (i=0; i < dacl->num_aces; i++) { // second deref if dacl is NULL ## Crash _samr_SetSecurity+146: mov 0x20(%rax),%r14 // rax = sd = 0x0, offset 0x20 = ->dacl Client receives STATUS_PIPE_DISCONNECTED (rpcd_lsad worker dies). ## Trigger Send SamrSetSecurityObject (opnum 2) with a valid user handle and `sec_desc_buf { sd_size=0, sd=NULL }`.
Tristan suggests CVSS 3.1: 6.5 (AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:N/A:H) which is the classic "authenticated user can crash the server" score. This looks plausible.
Thanks for confirming. The lab PoC is available if useful for regression testing. Happy to submit a patch -- the fix is a two-line NULL check for both sd and dacl before the dereference at line 726.
Created attachment 19022 [details] Patch proposal - s3:samr: check for NULL security descriptor in _samr_SetSecurity()
Hi, Just to follow up on this case. Please also let me know if I can do anything else. Best, Tristan
Moving this to the non-embargo pool, per our rule: https://wiki.samba.org/index.php/Samba_Security_Process 4b.) Even for a CVSSv3.1 of more than 5.0 we do not do a security release where the impact is: * Only a Denial of Service (not a use after free) and the automated internal process restart will allow other clients to reconnect eventually. For clarity, the shared LDAP or KDC process in the AD DC will restart, so a simple NULL de-reference crash there would not trigger a release. However a similar issue in the main smbd listener preventing new connections being established may require a release. This is similar to, but an extension of, our existing rule that crashing the smbd belonging to the connecting user is not a DoS. Therefore I think this can just be patched in master.
Removing embargo so patch can be submitted per https://wiki.samba.org/index.php/Contribute and following https://www.samba.org/samba/devel/copyright-policy.html Tristan, Thank you for your report, can you submit the MR please?
This bug was referenced in samba master: d3620cfd6bf3aa4243ae35fbc410711fb8e6519f