From 091b0265fe42878d676def5d4f5b4f8f3977b0e2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 5 Jun 2023 18:02:20 +0200 Subject: [PATCH] CVE-2023-34968: mdssvc: return a fake share path Instead of returning the real server-side absolute path of shares and search results, return a fake absolute path replacing the path of the share with the share name, iow for a share "test" with a server-side path of "/foo/bar", we previously returned /foo/bar and /foo/bar/search/result and now return /test and /test/search/result BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher Upstream-Status: Backport [https://github.com/samba-team/samba/commit/091b0265fe42878d676def5d4f5b4f8f3977b0e2] CVE: CVE-2023-34968 Signed-off-by: Archana Polampalli --- source3/lib/util_path.c | 52 ++++++++++++++++++++ source3/lib/util_path.h | 5 ++ source3/rpc_server/mdssvc/mdssvc.c | 60 +++++++++++++++++++++-- source3/rpc_server/mdssvc/mdssvc.h | 1 + source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 17 +++++-- 6 files changed, 128 insertions(+), 7 deletions(-) mode change 100755 => 100644 source3/libads/ldap.c diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c index c34b734..5b5a51c 100644 --- a/source3/lib/util_path.c +++ b/source3/lib/util_path.c @@ -21,8 +21,10 @@ * along with this program. If not, see . */ +#include "includes.h" #include "replace.h" #include +#include "lib/util/debug.h" #include "lib/util/samba_util.h" #include "lib/util_path.h" @@ -210,3 +212,53 @@ char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *pathname_in) *p++ = '\0'; return pathname; } + +/* + * Take two absolute paths, figure out if "subdir" is a proper + * subdirectory of "parent". Return the component relative to the + * "parent" without the potential "/". Take care of "parent" + * possibly ending in "/". + */ +bool subdir_of(const char *parent, + size_t parent_len, + const char *subdir, + const char **_relative) +{ + const char *relative = NULL; + bool matched; + + SMB_ASSERT(parent[0] == '/'); + SMB_ASSERT(subdir[0] == '/'); + + if (parent_len == 1) { + /* + * Everything is below "/" + */ + *_relative = subdir+1; + return true; + } + + if (parent[parent_len-1] == '/') { + parent_len -= 1; + } + + matched = (strncmp(subdir, parent, parent_len) == 0); + if (!matched) { + return false; + } + + relative = &subdir[parent_len]; + + if (relative[0] == '\0') { + *_relative = relative; /* nothing left */ + return true; + } + + if (relative[0] == '/') { + /* End of parent must match a '/' in subdir. */ + *_relative = relative+1; + return true; + } + + return false; +} diff --git a/source3/lib/util_path.h b/source3/lib/util_path.h index 3e7d04d..6d2155a 100644 --- a/source3/lib/util_path.h +++ b/source3/lib/util_path.h @@ -31,5 +31,10 @@ char *lock_path(TALLOC_CTX *mem_ctx, const char *name); char *state_path(TALLOC_CTX *mem_ctx, const char *name); char *cache_path(TALLOC_CTX *mem_ctx, const char *name); char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path); +bool subdir_of(const char *parent, + size_t parent_len, + const char *subdir, + const char **_relative); + #endif diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c index 19257e8..d442d8d 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -520,11 +520,14 @@ static bool inode_map_add(struct sl_query *slq, bool mds_add_result(struct sl_query *slq, const char *path) { struct smb_filename *smb_fname = NULL; + char *fake_path = NULL; + const char *relative = NULL; struct stat_ex sb; uint32_t attr; uint64_t ino64; int result; NTSTATUS status; + bool sub; bool ok; /* @@ -610,6 +613,17 @@ bool mds_add_result(struct sl_query *slq, const char *path) } } + sub = subdir_of(slq->mds_ctx->spath, + slq->mds_ctx->spath_len, + path, + &relative); + if (!sub) { + DBG_ERR("[%s] is not inside [%s]\n", + path, slq->mds_ctx->spath); + slq->state = SLQ_STATE_ERROR; + return false; + } + /* * Add inode number and filemeta to result set, this is what * we return as part of the result set of a query @@ -622,18 +636,30 @@ bool mds_add_result(struct sl_query *slq, const char *path) slq->state = SLQ_STATE_ERROR; return false; } + + fake_path = talloc_asprintf(slq, + "/%s/%s", + slq->mds_ctx->sharename, + relative); + if (fake_path == NULL) { + slq->state = SLQ_STATE_ERROR; + return false; + } + ok = add_filemeta(slq->mds_ctx, slq->reqinfo, slq->query_results->fm_array, - path, + fake_path, &sb); if (!ok) { DBG_ERR("add_filemeta error\n"); + TALLOC_FREE(fake_path); slq->state = SLQ_STATE_ERROR; return false; } - ok = inode_map_add(slq, ino64, path, &sb); + ok = inode_map_add(slq, ino64, fake_path, &sb); + TALLOC_FREE(fake_path); if (!ok) { DEBUG(1, ("inode_map_add error\n")); slq->state = SLQ_STATE_ERROR; @@ -840,6 +866,32 @@ static void slq_close_timer(struct tevent_context *ev, } } +/** + * Translate a fake scope from the client like /sharename/dir + * to the real server-side path, replacing the "/sharename" part + * with the absolute server-side path of the share. + **/ +static bool mdssvc_real_scope(struct sl_query *slq, const char *fake_scope) +{ + size_t sname_len = strlen(slq->mds_ctx->sharename); + size_t fake_scope_len = strlen(fake_scope); + + if (fake_scope_len < sname_len + 1) { + DBG_ERR("Short scope [%s] for share [%s]\n", + fake_scope, slq->mds_ctx->sharename); + return false; + } + + slq->path_scope = talloc_asprintf(slq, + "%s%s", + slq->mds_ctx->spath, + fake_scope + sname_len + 1); + if (slq->path_scope == NULL) { + return false; + } + return true; +} + /** * Begin a search query **/ @@ -946,8 +998,8 @@ static bool slrpc_open_query(struct mds_ctx *mds_ctx, goto error; } - slq->path_scope = talloc_strdup(slq, scope); - if (slq->path_scope == NULL) { + ok = mdssvc_real_scope(slq, scope); + if (!ok) { goto error; } diff --git a/source3/rpc_server/mdssvc/mdssvc.h b/source3/rpc_server/mdssvc/mdssvc.h index b3bd8b9..8434812 100644 --- a/source3/rpc_server/mdssvc/mdssvc.h +++ b/source3/rpc_server/mdssvc/mdssvc.h @@ -127,6 +127,7 @@ struct mds_ctx { int snum; const char *sharename; const char *spath; + size_t spath_len; struct connection_struct *conn; struct sl_query *query_list; /* list of active queries */ struct db_context *ino_path_map; /* dbwrap rbt for storing inode->path mappings */ diff --git a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c index 59e2a97..b20bd2a 100644 --- a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c +++ b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c @@ -121,6 +121,7 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r) loadparm_s3_global_substitution(); int snum; char *outpath = discard_const_p(char, r->out.share_path); + char *fake_path = NULL; char *path; NTSTATUS status; @@ -144,21 +145,31 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r) return; } + fake_path = talloc_asprintf(p->mem_ctx, "/%s", r->in.share_name); + if (fake_path == NULL) { + DBG_ERR("Couldn't create fake share path for %s\n", + r->in.share_name); + talloc_free(path); + p->fault_state = DCERPC_FAULT_CANT_PERFORM; + return; + } + status = create_mdssvc_policy_handle(p->mem_ctx, p, snum, r->in.share_name, path, r->out.handle); if (!NT_STATUS_IS_OK(status)) { - DBG_ERR("Couldn't create policy handle for %s\n", + DBG_ERR("Couldn't create path for %s\n", r->in.share_name); talloc_free(path); + talloc_free(fake_path); p->fault_state = DCERPC_FAULT_CANT_PERFORM; return; } - strlcpy(outpath, path, 1024); - talloc_free(path); + strlcpy(outpath, fake_path, 1024); + talloc_free(fake_path); return; } -- 2.40.0