The branch, v4-16-test has been updated via 78848f21a3e s3:client: Fix a use-after-free issue in smbclient via eeeb1a476f6 s3:script: Improve test_chdir_cache.sh via 4f9430f1260 s3:tests: Reformat test_chdir_cache.sh via 810ae90aa6c s3:params:lp_do_section - protect against NULL deref via b9d02e857b2 rpc_server:srvsvc - retrieve share ACL via root context via 104fcaa89f8 ctdb: Fix a use-after-free in run_proc from cb4cbfc83fc VERSION: Bump version up to Samba 4.16.9...
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-16-test - Log ----------------------------------------------------------------- commit 78848f21a3ecefdc6689c2794b166981eb517205 Author: Andreas Schneider <a...@samba.org> Date: Thu Dec 22 10:31:11 2022 +0100 s3:client: Fix a use-after-free issue in smbclient Detected by make test TESTS="samba3.blackbox.chdir-cache" with an optimized build or with AddressSanitizer. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15268 Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> (cherry picked from commit 9c707b4be27e2a6f79886d3ec8b5066c922b99bd) Autobuild-User(v4-16-test): Jule Anger <jan...@samba.org> Autobuild-Date(v4-16-test): Tue Jan 3 19:19:57 UTC 2023 on sn-devel-184 commit eeeb1a476f60b55f27083cdbe51c540ed4d86cc6 Author: Andreas Schneider <a...@samba.org> Date: Thu Dec 22 10:36:02 2022 +0100 s3:script: Improve test_chdir_cache.sh BUG: https://bugzilla.samba.org/show_bug.cgi?id=15268 Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> (cherry picked from commit 0d1961267cd9e8f1158a407c5d135514c363f37e) commit 4f9430f1260b9bd72a4d8f6a0030f6d139331449 Author: Andreas Schneider <a...@samba.org> Date: Fri Apr 22 15:34:08 2022 +0200 s3:tests: Reformat test_chdir_cache.sh shfmt -f source3/script/| xargs shfmt -w -p -i 0 -fn Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 810ae90aa6c34694c692015bb9f47f56ada811d2 Author: Andrew Walker <awal...@ixsystems.com> Date: Mon Dec 19 08:17:47 2022 -0500 s3:params:lp_do_section - protect against NULL deref iServiceIndex may indicate an empty slot in the ServicePtrs array. In this case, lpcfg_serivce_ok(ServicePtrs[iServiceIndex]) may trigger a NULL deref and crash. Skipping the check here will cause a scan of the array in add_a_service() and the NULL slot will be used safely. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15267 Signed-off-by: Andrew Walker <awal...@ixsystems.com> Reviewed-by: Jeremy Allison <j...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Tue Dec 20 18:49:54 UTC 2022 on sn-devel-184 (cherry picked from commit 5b19288949e97a5af742ff2719992d56f21e364a) commit b9d02e857b2cd95a207e06e5c29daa23c45d180d Author: Andrew <awal...@ixsystems.com> Date: Fri Dec 16 08:16:10 2022 -0800 rpc_server:srvsvc - retrieve share ACL via root context share_info.tdb has permissions of 0o600 and so we need to become_root() prior to retrieving the security info. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15265 Signed-off-by: Andrew Walker <awal...@ixsystems.com> Reviewed-by: Jeremy Allison <j...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Mon Dec 19 20:41:15 UTC 2022 on sn-devel-184 (cherry picked from commit 80c0b416892bfacc0d919fe032461748d7962f05) commit 104fcaa89f81d1a66735c1b85830e2e85460d1e0 Author: Volker Lendecke <v...@samba.org> Date: Fri Sep 30 17:02:41 2022 +0200 ctdb: Fix a use-after-free in run_proc If you happen to talloc_free(run_ctx) before all the tevent_req's hanging off it, you run into the following: ==495196== Invalid read of size 8 ==495196== at 0x10D757: run_proc_state_destructor (run_proc.c:413) ==495196== by 0x488F736: _tc_free_internal (talloc.c:1158) ==495196== by 0x488FBDD: _talloc_free_internal (talloc.c:1248) ==495196== by 0x4890F41: _talloc_free (talloc.c:1792) ==495196== by 0x48538B1: tevent_req_received (tevent_req.c:293) ==495196== by 0x4853429: tevent_req_destructor (tevent_req.c:129) ==495196== by 0x488F736: _tc_free_internal (talloc.c:1158) ==495196== by 0x4890AF6: _tc_free_children_internal (talloc.c:1669) ==495196== by 0x488F967: _tc_free_internal (talloc.c:1184) ==495196== by 0x488FBDD: _talloc_free_internal (talloc.c:1248) ==495196== by 0x4890F41: _talloc_free (talloc.c:1792) ==495196== by 0x10DE62: main (run_proc_test.c:86) ==495196== Address 0x55b77f8 is 152 bytes inside a block of size 160 free'd ==495196== at 0x48399AB: free (vg_replace_malloc.c:538) ==495196== by 0x488FB25: _tc_free_internal (talloc.c:1222) ==495196== by 0x488FBDD: _talloc_free_internal (talloc.c:1248) ==495196== by 0x4890F41: _talloc_free (talloc.c:1792) ==495196== by 0x10D315: run_proc_context_destructor (run_proc.c:329) ==495196== by 0x488F736: _tc_free_internal (talloc.c:1158) ==495196== by 0x488FBDD: _talloc_free_internal (talloc.c:1248) ==495196== by 0x4890F41: _talloc_free (talloc.c:1792) ==495196== by 0x10DE62: main (run_proc_test.c:86) ==495196== Block was alloc'd at ==495196== at 0x483877F: malloc (vg_replace_malloc.c:307) ==495196== by 0x488EAD9: __talloc_with_prefix (talloc.c:783) ==495196== by 0x488EC73: __talloc (talloc.c:825) ==495196== by 0x488F0FC: _talloc_named_const (talloc.c:982) ==495196== by 0x48925B1: _talloc_zero (talloc.c:2421) ==495196== by 0x10C8F2: proc_new (run_proc.c:61) ==495196== by 0x10D4C9: run_proc_send (run_proc.c:381) ==495196== by 0x10DDF6: main (run_proc_test.c:79) This happens because run_proc_context_destructor() directly does a talloc_free() on the struct proc_context's and not the enclosing tevent_req's. run_proc_kill() makes sure that we don't follow proc->req, but it forgets the "state->proc", which is free()'ed, but later dereferenced in run_proc_state_destructor(). This is an attempt at a quick fix, I believe we should convert run_proc_context->plist into an array of tevent_req's, so that we can properly TALLOC_FREE() according to the "natural" hierarchy and not just pull an arbitrary thread out of that heap. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15269 Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Martin Schwenke <mar...@meltin.net> Autobuild-User(master): Volker Lendecke <v...@samba.org> Autobuild-Date(master): Thu Oct 6 15:10:20 UTC 2022 on sn-devel-184 (cherry picked from commit 688be0177b04d04709813a02ae6da1e983ac25dd) ----------------------------------------------------------------------- Summary of changes: ctdb/common/run_proc.c | 5 ++-- source3/client/client.c | 5 ++-- source3/param/loadparm.c | 2 +- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 17 ++++++++++-- source3/script/tests/test_chdir_cache.sh | 46 +++++++++++++++++++------------ 5 files changed, 49 insertions(+), 26 deletions(-) Changeset truncated at 500 lines: diff --git a/ctdb/common/run_proc.c b/ctdb/common/run_proc.c index d55af6c3a1e..84bc343ba1f 100644 --- a/ctdb/common/run_proc.c +++ b/ctdb/common/run_proc.c @@ -408,10 +408,10 @@ struct tevent_req *run_proc_send(TALLOC_CTX *mem_ctx, static int run_proc_state_destructor(struct run_proc_state *state) { /* Do not get rid of the child process if timeout has occurred */ - if (state->proc->req != NULL) { + if ((state->proc != NULL) && (state->proc->req != NULL)) { state->proc->req = NULL; DLIST_REMOVE(state->run_ctx->plist, state->proc); - talloc_free(state->proc); + TALLOC_FREE(state->proc); } return 0; @@ -439,6 +439,7 @@ static void run_proc_kill(struct tevent_req *req) req, struct run_proc_state); state->proc->req = NULL; + state->proc = NULL; state->result.sig = SIGKILL; diff --git a/source3/client/client.c b/source3/client/client.c index 69b7c9022c2..607be92bba3 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -5126,10 +5126,11 @@ static int cmd_tcon(void) return -1; } - talloc_free(sharename); - d_printf("tcon to %s successful, tid: %u\n", sharename, cli_state_get_tid(cli)); + + talloc_free(sharename); + return 0; } diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 6d567a9e7e5..1cdfbb70276 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -2883,7 +2883,7 @@ bool lp_do_section(const char *pszSectionName, void *userdata) /* if we have a current service, tidy it up before moving on */ bRetval = true; - if (iServiceIndex >= 0) + if ((iServiceIndex >= 0) && (ServicePtrs[iServiceIndex] != NULL)) bRetval = lpcfg_service_ok(ServicePtrs[iServiceIndex]); /* if all is still well, move to the next record in the services array */ diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index f0686a411e1..3f268d66080 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -536,6 +536,7 @@ static bool is_hidden_share(int snum) static bool is_enumeration_allowed(struct pipes_struct *p, int snum) { + bool allowed; struct dcesrv_call_state *dce_call = p->dce_call; struct auth_session_info *session_info = dcesrv_call_session_info(dce_call); @@ -552,9 +553,19 @@ static bool is_enumeration_allowed(struct pipes_struct *p, return false; } - return share_access_check(session_info->security_token, - lp_servicename(talloc_tos(), lp_sub, snum), - FILE_READ_DATA, NULL); + + /* + * share_access_check() must be opened as root + * because it ultimately gets a R/W db handle on share_info.tdb + * which has 0o600 permissions + */ + become_root(); + allowed = share_access_check(session_info->security_token, + lp_servicename(talloc_tos(), lp_sub, snum), + FILE_READ_DATA, NULL); + unbecome_root(); + + return allowed; } /**************************************************************************** diff --git a/source3/script/tests/test_chdir_cache.sh b/source3/script/tests/test_chdir_cache.sh index 6287d17354a..c649d2b07b3 100755 --- a/source3/script/tests/test_chdir_cache.sh +++ b/source3/script/tests/test_chdir_cache.sh @@ -8,16 +8,21 @@ # Copyright (C) 2021 Jeremy Allison if [ $# -lt 5 ]; then - echo Usage: test_chdir_user.sh \ - --configfile=SERVERCONFFILE SMBCLIENT SMBCONTROL SERVER SHARE -exit 1 + echo Usage: test_chdir_user.sh \ + --configfile=SERVERCONFFILE SMBCLIENT SMBCONTROL SERVER SHARE + exit 1 fi -CONF=$1; shift 1 -SMBCLIENT=$1; shift 1 -SMBCONTROL=$1; shift 1 -SERVER=$1; shift 1 -SHARE=$1; shift 1 +CONF=$1 +shift 1 +SMBCLIENT=$1 +shift 1 +SMBCONTROL=$1 +shift 1 +SERVER=$1 +shift 1 +SHARE=$1 +shift 1 # Do not let deprecated option warnings muck this up SAMBA_DEPRECATED_SUPPRESS=1 @@ -28,7 +33,7 @@ conf_dir=$(dirname ${SERVERCONFFILE}) log_file=${conf_dir}/../smbd_test.log error_inject_conf=${conf_dir}/error_inject.conf -> ${error_inject_conf} +rm -f ${error_inject_conf} incdir=$(dirname $0)/../../../testprogs/blackbox . $incdir/subunit.sh @@ -40,15 +45,16 @@ cd $SELFTEST_TMPDIR || exit 1 rm -f smbclient-stdin smbclient-stdout smbclient-stderr mkfifo smbclient-stdin smbclient-stdout smbclient-stderr -CLI_FORCE_INTERACTIVE=1; export CLI_FORCE_INTERACTIVE +CLI_FORCE_INTERACTIVE=1 +export CLI_FORCE_INTERACTIVE ${SMBCLIENT} //${SERVER}/${SHARE} ${CONF} -U${USER}%${PASSWORD} \ - < smbclient-stdin > smbclient-stdout 2>smbclient-stderr & + <smbclient-stdin >smbclient-stdout 2>smbclient-stderr & CLIENT_PID=$! # Count the number of chdir_current_service: vfs_ChDir.*failed: Permission denied # errors that are already in the log (should be zero). -num_errs=`grep "chdir_current_service: vfs_ChDir.*failed: Permission denied" ${log_file} | wc -l` +num_errs=$(grep "chdir_current_service: vfs_ChDir.*failed: Permission denied" ${log_file} | wc -l) sleep 1 @@ -73,8 +79,10 @@ echo "tcon ${SHARE}" >&100 head -n 4 <&101 # Ensure any chdir will give EACCESS. -echo "error_inject:chdir = EACCES" > ${error_inject_conf} -${SMBCONTROL} ${CONF} 0 reload-config +echo "error_inject:chdir = EACCES" >${error_inject_conf} +testit "reload config 1" \ + "${SMBCONTROL}" "${CONF}" smbd reload-config || + failed=$((failed + 1)) sleep 1 @@ -88,15 +96,17 @@ kill ${CLIENT_PID} rm -f smbclient-stdin smbclient-stdout smbclient-stderr # Remove the chdir inject. -> ${error_inject_conf} -${SMBCONTROL} ${CONF} 0 reload-config +rm -f ${error_inject_conf} +testit "reload config 2" \ + "${SMBCONTROL}" "${CONF}" smbd reload-config || + failed=$((failed + 1)) # Now look for chdir_current_service: vfs_ChDir.*failed: Permission denied # in the smb log. There should be one more than before. -num_errs1=`grep "chdir_current_service: vfs_ChDir.*failed: Permission denied" ${log_file} | wc -l` +num_errs1=$(grep "chdir_current_service: vfs_ChDir.*failed: Permission denied" ${log_file} | wc -l) testit "Verify we got at least one chdir error" \ - test $num_errs1 -gt $num_errs || failed=$(expr $failed + 1) + test $num_errs1 -gt $num_errs || failed=$(expr $failed + 1) testok $0 $failed -- Samba Shared Repository