reply_sesssetup_and_X() in source3/smbd/smb1_sesssetup.c contains: char *tmp; ...; p += srvstr_pull_req_talloc(state, req, &tmp, p, STR_TERMINATE); state->user = tmp ? tmp : ""; But srvstr_pull_req_talloc() returns without setting tmp if there is nothing left in the input string: size_t srvstr_pull_req_talloc(TALLOC_CTX *ctx, struct smb_request *req, char **dest, const uint8_t *src, int flags) { ssize_t bufrem = smbreq_bufrem(req, src); if (bufrem == 0) { return 0; } This will cause subsequent use of state->user[] to dereference whatever garbage value happened to be in tmp. A client can make this happen by sending a SMB_COM_SESSION_SETUP_ANDX message whose SMB Data ends at just the point where the AccountName should start.
Again, can you attach your test code please ? That will make it much easier for us to write a regression test case. Thanks !
Created attachment 17984 [details] demo client program
(In reply to Jeremy Allison from comment #1) I've added a demo client. To see a crash, modify reply_sesssetup_and_X to explicitly initialize tmp to some illegal address.
Another place this comes up is in reply_trans() in smb1_ipc.c, where this call will leave state->name uninitialized if the SMB_Data part of an SMB_COM_NT_TRANSACT ends just where the \pipe\ name ought to start: srvstr_pull_req_talloc(state, req, &state->name, req->buf, STR_TERMINATE);
Thanks, I'll do a thorough review of all code that matches this pattern and try and fix them all at once. This looks like a self-DOS though, not a security issue I think. Can't see any way this can be exploited.
I have a test case reproducer for this that crashes smbd. Now I just need to figure out why the test harness isn't noticing that smbd crashed :-).
Created attachment 18042 [details] Patch under gitlab ci.
MR: https://gitlab.com/samba-team/samba/-/merge_requests/3220
Created attachment 18044 [details] WIP patches for master.
This bug was referenced in samba master: e7bf94b4e3a7f994aa6f0b859089c5add2ad380f 963fd8aa9b76361ab9aeb63307773f2498b17879 9220c45cc191b34e293190f6a923ba463edd5db9 5bc50d2ea4444244721e72b4264311c7005d2f3c 5379b8d557a9a16b81eafb87b60b81debc4bfccb
Created attachment 18046 [details] git-am fix for 4.19.next, 4.18.next. Cherry-picked from master.
Created attachment 18047 [details] git-am fix for 4.17.next Back-ported from master.
Pushed to autobuild-v4-{19,18,17}-test.
This bug was referenced in samba v4-19-test: 7b84b08693bab381634c3a0d28212fd7706b74dc 60cbe064ba1fcbc3b53c54d2ba4a04cf1f8f91ce 116c740cb9c30bc2471dcb7ba2a771c86b02061a a36f30498db60d876467f9df87231838bab7a038 18bd1f75d4f03e755020151925e08ebac3866d0c
This bug was referenced in samba v4-17-test: bce87c64b717bbb7ad78063f5918e715403f49fd ec8887be3f6f128911be4855c6ad302140d4d5ff 3a123fbbe86dbc8dba8612095554bff8309ab381 b958e82d0b67654a76fc95edde646c6ea8f99ae1 0dbba5f655f6e84b07f04c8e23719a72c67594ed
This bug was referenced in samba v4-18-test: 8e31fd2d5998d08f66bad617aecbc6dc4a9490cf 6fef976770c596e5663f49a3cf194b0e6cdf83e6 4c27dfe322c28e01a5657e1e707000474d2343f9 f025f51ac5c363c37f4a80baa06c40106ae2bdae e6c0d4f122d8083273009d0f61e099bb34fbcf51
This bug was referenced in samba v4-18-stable (Release samba-4.18.6): 8e31fd2d5998d08f66bad617aecbc6dc4a9490cf 6fef976770c596e5663f49a3cf194b0e6cdf83e6 4c27dfe322c28e01a5657e1e707000474d2343f9 f025f51ac5c363c37f4a80baa06c40106ae2bdae e6c0d4f122d8083273009d0f61e099bb34fbcf51
Closing out bug report. Thanks!
This bug was referenced in samba v4-19-stable (Release samba-4.19.0rc3): 7b84b08693bab381634c3a0d28212fd7706b74dc 60cbe064ba1fcbc3b53c54d2ba4a04cf1f8f91ce 116c740cb9c30bc2471dcb7ba2a771c86b02061a a36f30498db60d876467f9df87231838bab7a038 18bd1f75d4f03e755020151925e08ebac3866d0c
This bug was referenced in samba v4-17-stable (Release samba-4.17.11): bce87c64b717bbb7ad78063f5918e715403f49fd ec8887be3f6f128911be4855c6ad302140d4d5ff 3a123fbbe86dbc8dba8612095554bff8309ab381 b958e82d0b67654a76fc95edde646c6ea8f99ae1 0dbba5f655f6e84b07f04c8e23719a72c67594ed