From 05199f0ceed21b6c60d489e5879ffd24316f50ea Mon Sep 17 00:00:00 2001 From: Marios Makassikis Date: Mon, 7 Nov 2022 13:42:53 +0100 Subject: [PATCH 15/17] ksmbd-tools: mountd: fix out of bounds access in set_domain_name() smb_sid_to_string() does not check bounds when writing to its output buffer. Additionally, it does not null terminate it, so the call to strlen() in set_domain_name() can go past the end of the buffer. Replace calls to strcpy() with snprintf() to ensure null termination and detect overflows. Signed-off-by: Marios Makassikis --- include/smbacl.h | 2 +- mountd/rpc_lsarpc.c | 3 +- mountd/smbacl.c | 69 +++++++++++++++++++++++++++++---------------- 3 files changed, 47 insertions(+), 27 deletions(-) diff --git a/include/smbacl.h b/include/smbacl.h index a5551c91b241..5bf3070f57bb 100644 --- a/include/smbacl.h +++ b/include/smbacl.h @@ -77,5 +77,5 @@ int smb_write_sid(struct ksmbd_dcerpc *dce, const struct smb_sid *src); void smb_copy_sid(struct smb_sid *dst, const struct smb_sid *src); int smb_compare_sids(const struct smb_sid *ctsid, const struct smb_sid *cwsid); int build_sec_desc(struct ksmbd_dcerpc *dce, __u32 *secdesclen, int rid); -int set_domain_name(struct smb_sid *sid, char *domain, int *type); +int set_domain_name(struct smb_sid *sid, char *domain, size_t domain_len, int *type); #endif /* __KSMBD_SMBACL_H__ */ diff --git a/mountd/rpc_lsarpc.c b/mountd/rpc_lsarpc.c index 2bcbf7feaa63..daf9b45253ac 100644 --- a/mountd/rpc_lsarpc.c +++ b/mountd/rpc_lsarpc.c @@ -304,7 +304,8 @@ static int lsarpc_lookup_sid2_invoke(struct ksmbd_rpc_pipe *pipe) ni->user = usm_lookup_user(passwd->pw_name); ni->index = i + 1; - if (set_domain_name(&ni->sid, ni->domain_str, &ni->type)) + if (set_domain_name(&ni->sid, ni->domain_str, + sizeof(ni->domain_str), &ni->type)) ni->index = -1; pipe->entries = g_array_append_val(pipe->entries, ni); diff --git a/mountd/smbacl.c b/mountd/smbacl.c index 936094f0c444..d23deadd58cf 100644 --- a/mountd/smbacl.c +++ b/mountd/smbacl.c @@ -137,29 +137,29 @@ int smb_compare_sids(const struct smb_sid *ctsid, const struct smb_sid *cwsid) return 0; /* sids compare/match */ } -static void smb_sid_to_string(char *domain, struct smb_sid *sid) +static int smb_sid_to_string(char *domain, size_t domain_len, + struct smb_sid *sid) { - char str[PATH_MAX]; - int i, domain_len, len; + int i, len; - domain_len = sprintf(domain, "S-"); - len = sprintf(str, "%i", (int)sid->revision); - strncpy(&domain[domain_len], str, len); - domain_len += len; - domain[domain_len++] = '-'; - len = sprintf(str, "%i", (int)sid->authority[5]); - strncpy(&domain[domain_len], str, len); - domain_len += len; + len = snprintf(domain, domain_len, "S-%i-%i", (int)sid->revision, + (int)sid->authority[5]); + + if (len < 0 || len > domain_len) + return -ENOMEM; for (i = 0; i < sid->num_subauth; i++) { - domain[domain_len++] = '-'; - len = sprintf(str, "%u", sid->sub_auth[i]); - strncpy(&domain[domain_len], str, len); - domain_len += len; + len += snprintf(domain + len, domain_len - len, "-%u", + sid->sub_auth[i]); + if (len < 0 || len > domain_len) + return -ENOMEM; } + + return len; } -int set_domain_name(struct smb_sid *sid, char *domain, int *type) +int set_domain_name(struct smb_sid *sid, char *domain, size_t domain_len, + int *type) { int ret = 0; char domain_string[NAME_MAX] = {0}; @@ -171,25 +171,44 @@ int set_domain_name(struct smb_sid *sid, char *domain, int *type) if (gethostname(domain_string, NAME_MAX)) return -ENOMEM; - domain_name = g_ascii_strup(domain_string, - strlen(domain_string)); + domain_name = g_ascii_strup(domain_string, -1); if (!domain_name) return -ENOMEM; - strcpy(domain, domain_name); + + ret = snprintf(domain, domain_len, "%s", domain_name); + if (ret < 0 || ret >= domain_len) + return -ENOMEM; + *type = SID_TYPE_USER; } else if (!smb_compare_sids(sid, &sid_unix_users)) { - strcpy(domain, "Unix User"); + ret = snprintf(domain, domain_len, "Unix User"); + if (ret < 0 || ret >= domain_len) + return -ENOMEM; + *type = SID_TYPE_USER; } else if (!smb_compare_sids(sid, &sid_unix_groups)) { - strcpy(domain, "Unix Group"); + ret = snprintf(domain, domain_len, "Unix Group"); + if (ret < 0 || ret >= domain_len) + return -ENOMEM; + *type = SID_TYPE_GROUP; } else { - smb_sid_to_string(domain_string, sid); - domain_name = g_ascii_strup(domain_string, - strlen(domain_string)); + ret = smb_sid_to_string(domain_string, sizeof(domain_string), + sid); + if (ret < 0) + return ret; + + if (ret > domain_len) + return -ENOMEM; + + domain_name = g_ascii_strup(domain_string, -1); if (!domain_name) return -ENOMEM; - strcpy(domain, domain_name); + + ret = snprintf(domain, domain_len, "%s", domain_name); + if (ret < 0 || ret >= domain_len) + return -ENOMEM; + *type = SID_TYPE_UNKNOWN; ret = -ENOENT; } -- 2.25.1