From 83a1b88631df374b7bc741efa8e9ef4eeade790b Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 9 Jun 2020 11:52:50 +1000 Subject: [PATCH 1/3] util: Simplify input validation It appears that snprintf(3) is being used for input validation. However, this seems like overkill because it causes szPath to be copied an extra time. The mostly likely protections being sought here, according to https://cwe.mitre.org/data/definitions/20.html, look to be DoS attacks involving CPU and memory usage. A simpler check that uses strnlen(3) can mitigate against both of these and is simpler. Signed-off-by: Martin Schwenke Reviewed-by: Volker Lendecke Reviewed-by: Bjoern Jacke (cherry picked from commit 922bce2668994dd2a5988c17060f977e9bb0c229) --- lib/util/util_paths.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/util/util_paths.c b/lib/util/util_paths.c index c0ee5c32c30..dec91772d9e 100644 --- a/lib/util/util_paths.c +++ b/lib/util/util_paths.c @@ -69,21 +69,20 @@ static char *get_user_home_dir(TALLOC_CTX *mem_ctx) struct passwd pwd = {0}; struct passwd *pwdbuf = NULL; char buf[NSS_BUFLEN_PASSWD] = {0}; + size_t len; int rc; rc = getpwuid_r(getuid(), &pwd, buf, NSS_BUFLEN_PASSWD, &pwdbuf); if (rc != 0 || pwdbuf == NULL ) { - int len_written; const char *szPath = getenv("HOME"); if (szPath == NULL) { return NULL; } - len_written = snprintf(buf, sizeof(buf), "%s", szPath); - if (len_written >= sizeof(buf) || len_written < 0) { - /* Output was truncated or an error. */ + len = strnlen(szPath, PATH_MAX); + if (len >= PATH_MAX) { return NULL; } - return talloc_strdup(mem_ctx, buf); + return talloc_strdup(mem_ctx, szPath); } return talloc_strdup(mem_ctx, pwd.pw_dir); -- 2.20.2 From 1af4cb57afa388ecd4f42330a04a9475d6cf4c11 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 5 Jun 2020 21:52:23 +1000 Subject: [PATCH 2/3] util: Fix build on FreeBSD by avoiding NSS_BUFLEN_PASSWD NSS_BUFLEN_PASSWD is not defined on FreeBSD. Use sysconf(_SC_GETPW_R_SIZE_MAX) instead, as per POSIX. Use a dynamically allocated buffer instead of trying to cram all of the logic into the declarations. This will come in useful later anyway. Signed-off-by: Martin Schwenke Reviewed-by: Volker Lendecke Reviewed-by: Bjoern Jacke (cherry picked from commit 847208cd8ac68c4c7d1dae63767820db1c69292b) --- lib/util/util_paths.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/util/util_paths.c b/lib/util/util_paths.c index dec91772d9e..9bc6df37e5d 100644 --- a/lib/util/util_paths.c +++ b/lib/util/util_paths.c @@ -68,24 +68,41 @@ static char *get_user_home_dir(TALLOC_CTX *mem_ctx) { struct passwd pwd = {0}; struct passwd *pwdbuf = NULL; - char buf[NSS_BUFLEN_PASSWD] = {0}; + char *buf = NULL; + char *out = NULL; + long int initlen; size_t len; int rc; - rc = getpwuid_r(getuid(), &pwd, buf, NSS_BUFLEN_PASSWD, &pwdbuf); + initlen = sysconf(_SC_GETPW_R_SIZE_MAX); + if (initlen == -1) { + len = 1024; + } else { + len = (size_t)initlen; + } + buf = talloc_size(mem_ctx, len); + if (buf == NULL) { + return NULL; + } + + rc = getpwuid_r(getuid(), &pwd, buf, len, &pwdbuf); if (rc != 0 || pwdbuf == NULL ) { const char *szPath = getenv("HOME"); if (szPath == NULL) { - return NULL; + goto done; } len = strnlen(szPath, PATH_MAX); if (len >= PATH_MAX) { return NULL; } - return talloc_strdup(mem_ctx, szPath); + out = talloc_strdup(mem_ctx, szPath); + goto done; } - return talloc_strdup(mem_ctx, pwd.pw_dir); + out = talloc_strdup(mem_ctx, pwd.pw_dir); +done: + TALLOC_FREE(buf); + return out; } char *path_expand_tilde(TALLOC_CTX *mem_ctx, const char *d) -- 2.20.2 From afc6d6cc4ee97c0d8003f73bc93f06fdb9786fc9 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 5 Jun 2020 22:05:42 +1000 Subject: [PATCH 3/3] util: Reallocate larger buffer if getpwuid_r() returns ERANGE Signed-off-by: Martin Schwenke Reviewed-by: Volker Lendecke Reviewed-by: Bjoern Jacke Autobuild-User(master): Martin Schwenke Autobuild-Date(master): Tue Jun 9 21:07:24 UTC 2020 on sn-devel-184 (cherry picked from commit ddac6b2eb4adaec8fc5e25ca07387d2b9417764c) --- lib/util/util_paths.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/util/util_paths.c b/lib/util/util_paths.c index 9bc6df37e5d..72cc0aab8de 100644 --- a/lib/util/util_paths.c +++ b/lib/util/util_paths.c @@ -86,6 +86,19 @@ static char *get_user_home_dir(TALLOC_CTX *mem_ctx) } rc = getpwuid_r(getuid(), &pwd, buf, len, &pwdbuf); + while (rc == ERANGE) { + size_t newlen = 2 * len; + if (newlen < len) { + /* Overflow */ + goto done; + } + len = newlen; + buf = talloc_realloc_size(mem_ctx, buf, len); + if (buf == NULL) { + goto done; + } + rc = getpwuid_r(getuid(), &pwd, buf, len, &pwdbuf); + } if (rc != 0 || pwdbuf == NULL ) { const char *szPath = getenv("HOME"); if (szPath == NULL) { -- 2.20.2