From 9363dc1e82746b94a09956bbf82e2b10e425b0c4 Mon Sep 17 00:00:00 2001 From: Benjamin Kaduk Date: Wed, 22 Oct 2014 23:57:18 -0400 Subject: [PATCH] Pull patches from gerrit for realpath() issues realpath() insists on having a sufficiently large buffer passed in; it will stomp on all the memory and cause an application crash if the provided allocation is too small. --- debian/changelog | 2 + ...Fix-fixed-size-on-stack-path-buffers.patch | 272 ++++++++++++++++++ ...002-Tweak-AFSDIR_PATH_MAX-definition.patch | 43 +++ debian/patches/series | 2 + 4 files changed, 319 insertions(+) create mode 100644 debian/patches/0001-userok.c-Fix-fixed-size-on-stack-path-buffers.patch create mode 100644 debian/patches/0002-Tweak-AFSDIR_PATH_MAX-definition.patch create mode 100644 debian/patches/series diff --git a/debian/changelog b/debian/changelog index b4727c8d7..78ab2f748 100644 --- a/debian/changelog +++ b/debian/changelog @@ -19,6 +19,8 @@ openafs (1.6.10-1) UNRELEASED; urgency=medium - Support for cold shutdowns is removed. They generally resulted only in strange failures later on. * Add a systemd unit file for openafs-fileserver. + * Use heap-allocated buffers of sufficient length for realpath() in + userok.c. (Closes: #757378) -- Benjamin Kaduk Wed, 22 Oct 2014 13:10:59 -0400 diff --git a/debian/patches/0001-userok.c-Fix-fixed-size-on-stack-path-buffers.patch b/debian/patches/0001-userok.c-Fix-fixed-size-on-stack-path-buffers.patch new file mode 100644 index 000000000..e941b1999 --- /dev/null +++ b/debian/patches/0001-userok.c-Fix-fixed-size-on-stack-path-buffers.patch @@ -0,0 +1,272 @@ +From: Jeffrey Hutzelman +Date: Tue, 18 Jun 2013 12:35:36 -0400 +Subject: userok.c: Fix fixed-size on-stack path buffers + +Several functions in src/auth/userok.c construct pathnames in fixed +size buffers on their stacks. Those buffers are simultaneously too +small for the purpose for which they are used and too large to be +placed on the stack. This change replaces these fixed-size buffers +with dynamically-allocated buffers which are either exactly the right +size (due to asprintf) or have size AFSDIR_PATH_MAX. + +FIXES 130719 + +Reviewed-on: http://gerrit.openafs.org/9986 +Tested-by: BuildBot +Reviewed-by: Derrick Brashear +(cherry picked from commit 68e02987f62e1c507ddf7fd35847338b130c243d) + +This file has diverged quite substantially between master and 1.6.x, +so though it is marked as a "cherry-pick", this patch was substantially +rewritten for the 1.6 branch. In particular, we must use afs_asprintf() +since asprintf() is not available everywhere. + +Change-Id: Iac62cb8293e7b28b422e7401eccb1f26841aff66 +(cherry picked from commit 27c71d3f62b605ca30800769c9bea2c649337032) +--- + src/auth/userok.c | 104 +++++++++++++++++++++++++++++++++++++++--------------- + 1 file changed, 75 insertions(+), 29 deletions(-) + +diff --git a/src/auth/userok.c b/src/auth/userok.c +index 68b6fbf..3a9e493 100644 +--- a/src/auth/userok.c ++++ b/src/auth/userok.c +@@ -108,8 +108,8 @@ afsconf_SetNoAuthFlag(struct afsconf_dir *adir, int aflag) + int + afsconf_DeleteUser(struct afsconf_dir *adir, char *auser) + { +- char tbuffer[1024]; +- char nbuffer[1024]; ++ char *filename, *nfilename; ++ char *buffer; + FILE *tf; + FILE *nf; + int flag; +@@ -119,8 +119,17 @@ afsconf_DeleteUser(struct afsconf_dir *adir, char *auser) + struct stat tstat; + afs_int32 code; + ++ buffer = malloc(AFSDIR_PATH_MAX); ++ if (buffer == NULL) ++ return ENOMEM; ++ filename = malloc(AFSDIR_PATH_MAX); ++ if (filename == NULL) { ++ free(buffer); ++ return ENOMEM; ++ } ++ + LOCK_GLOBAL_MUTEX; +- strcompose(tbuffer, sizeof tbuffer, adir->name, "/", ++ strcompose(filename, AFSDIR_PATH_MAX, adir->name, "/", + AFSDIR_ULIST_FILE, NULL); + #ifndef AFS_NT40_ENV + { +@@ -129,64 +138,86 @@ afsconf_DeleteUser(struct afsconf_dir *adir, char *auser) + * of the temporary file will work even if UserList is a symlink + * into a different filesystem. + */ +- char resolved_path[1024]; +- +- if (realpath(tbuffer, resolved_path)) { +- strcpy(tbuffer, resolved_path); ++ nfilename = malloc(AFSDIR_PATH_MAX); ++ if (nfilename == NULL) { ++ UNLOCK_GLOBAL_MUTEX; ++ free(filename); ++ free(buffer); ++ return ENOMEM; ++ } ++ if (realpath(filename, nfilename)) { ++ free(filename); ++ filename = nfilename; ++ } else { ++ free(nfilename); + } + } + #endif /* AFS_NT40_ENV */ +- tf = fopen(tbuffer, "r"); ++ if (afs_asprintf(&nfilename, "%s.NXX", filename) < 0) { ++ UNLOCK_GLOBAL_MUTEX; ++ free(filename); ++ free(buffer); ++ return -1; ++ } ++ tf = fopen(filename, "r"); + if (!tf) { + UNLOCK_GLOBAL_MUTEX; ++ free(filename); ++ free(nfilename); ++ free(buffer); + return -1; + } +- code = stat(tbuffer, &tstat); ++ code = stat(filename, &tstat); + if (code < 0) { + UNLOCK_GLOBAL_MUTEX; ++ free(filename); ++ free(nfilename); ++ free(buffer); + return code; + } +- strcpy(nbuffer, tbuffer); +- strcat(nbuffer, ".NXX"); +- nf = fopen(nbuffer, "w+"); ++ nf = fopen(nfilename, "w+"); + if (!nf) { + fclose(tf); + UNLOCK_GLOBAL_MUTEX; ++ free(filename); ++ free(nfilename); ++ free(buffer); + return EIO; + } + flag = 0; + found = 0; + while (1) { + /* check for our user id */ +- tp = fgets(nbuffer, sizeof(nbuffer), tf); ++ tp = fgets(buffer, AFSDIR_PATH_MAX, tf); + if (tp == NULL) + break; +- code = sscanf(nbuffer, "%64s", tname); ++ code = sscanf(buffer, "%64s", tname); + if (code == 1 && strcmp(tname, auser) == 0) { + /* found the guy, don't copy to output file */ + found = 1; + } else { + /* otherwise copy original line to output */ +- fprintf(nf, "%s", nbuffer); ++ fprintf(nf, "%s", buffer); + } + } + fclose(tf); ++ free(buffer); + if (ferror(nf)) + flag = 1; + if (fclose(nf) == EOF) + flag = 1; +- strcpy(nbuffer, tbuffer); +- strcat(nbuffer, ".NXX"); /* generate new file name again */ + if (flag == 0) { + /* try the rename */ +- flag = renamefile(nbuffer, tbuffer); ++ flag = renamefile(nfilename, filename); + if (flag == 0) +- flag = chmod(tbuffer, tstat.st_mode); ++ flag = chmod(filename, tstat.st_mode); + } else +- unlink(nbuffer); ++ unlink(nfilename); + + /* finally, decide what to return to the caller */ + UNLOCK_GLOBAL_MUTEX; ++ free(filename); ++ free(nfilename); + if (flag) + return EIO; /* something mysterious went wrong */ + if (!found) +@@ -199,25 +230,30 @@ int + afsconf_GetNthUser(struct afsconf_dir *adir, afs_int32 an, char *abuffer, + afs_int32 abufferLen) + { +- char tbuffer[256]; ++ char *tbuffer; + FILE *tf; + char tname[64 + 1]; + char *tp; + int flag; + afs_int32 code; + ++ tbuffer = malloc(AFSDIR_PATH_MAX); ++ if (tbuffer == NULL) ++ return ENOMEM; ++ + LOCK_GLOBAL_MUTEX; +- strcompose(tbuffer, sizeof tbuffer, adir->name, "/", ++ strcompose(tbuffer, AFSDIR_PATH_MAX, adir->name, "/", + AFSDIR_ULIST_FILE, NULL); + tf = fopen(tbuffer, "r"); + if (!tf) { + UNLOCK_GLOBAL_MUTEX; ++ free(tbuffer); + return 1; + } + flag = 1; + while (1) { + /* check for our user id */ +- tp = fgets(tbuffer, sizeof(tbuffer), tf); ++ tp = fgets(tbuffer, AFSDIR_PATH_MAX, tf); + if (tp == NULL) + break; + code = sscanf(tbuffer, "%64s", tname); +@@ -230,6 +266,7 @@ afsconf_GetNthUser(struct afsconf_dir *adir, afs_int32 an, char *abuffer, + strcpy(abuffer, tname); + fclose(tf); + UNLOCK_GLOBAL_MUTEX; ++ free(tbuffer); + return flag; + } + +@@ -237,22 +274,28 @@ afsconf_GetNthUser(struct afsconf_dir *adir, afs_int32 an, char *abuffer, + static int + FindUser(struct afsconf_dir *adir, char *auser) + { +- char tbuffer[256]; ++ char *tbuffer; + bufio_p bp; + char tname[64 + 1]; + int flag; + afs_int32 code; + int rc; + +- strcompose(tbuffer, sizeof tbuffer, adir->name, "/", AFSDIR_ULIST_FILE, ++ tbuffer = malloc(AFSDIR_PATH_MAX); ++ if (tbuffer == NULL) ++ return 0; ++ ++ strcompose(tbuffer, AFSDIR_PATH_MAX, adir->name, "/", AFSDIR_ULIST_FILE, + NULL); + bp = BufioOpen(tbuffer, O_RDONLY, 0); +- if (!bp) ++ if (!bp) { ++ free(tbuffer); + return 0; ++ } + flag = 0; + while (1) { + /* check for our user id */ +- rc = BufioGets(bp, tbuffer, sizeof(tbuffer)); ++ rc = BufioGets(bp, tbuffer, AFSDIR_PATH_MAX); + if (rc < 0) + break; + code = sscanf(tbuffer, "%64s", tname); +@@ -262,6 +305,7 @@ FindUser(struct afsconf_dir *adir, char *auser) + } + } + BufioClose(bp); ++ free(tbuffer); + return flag; + } + +@@ -271,7 +315,7 @@ afsconf_AddUser(struct afsconf_dir *adir, char *aname) + { + FILE *tf; + afs_int32 code; +- char tbuffer[256]; ++ char *tbuffer; + + LOCK_GLOBAL_MUTEX; + if (FindUser(adir, aname)) { +@@ -279,9 +323,11 @@ afsconf_AddUser(struct afsconf_dir *adir, char *aname) + return EEXIST; /* already in the list */ + } + +- strcompose(tbuffer, sizeof tbuffer, adir->name, "/", AFSDIR_ULIST_FILE, ++ tbuffer = malloc(AFSDIR_PATH_MAX); ++ strcompose(tbuffer, AFSDIR_PATH_MAX, adir->name, "/", AFSDIR_ULIST_FILE, + NULL); + tf = fopen(tbuffer, "a+"); ++ free(tbuffer); + if (!tf) { + UNLOCK_GLOBAL_MUTEX; + return EIO; diff --git a/debian/patches/0002-Tweak-AFSDIR_PATH_MAX-definition.patch b/debian/patches/0002-Tweak-AFSDIR_PATH_MAX-definition.patch new file mode 100644 index 000000000..353880479 --- /dev/null +++ b/debian/patches/0002-Tweak-AFSDIR_PATH_MAX-definition.patch @@ -0,0 +1,43 @@ +From: Benjamin Kaduk +Date: Mon, 8 Sep 2014 13:47:33 -0400 +Subject: Tweak AFSDIR_PATH_MAX definition + +On recent Debian, we run into runtime errors in the test suite +because _POSIX_PATH_MAX is only 256, and that buffer is too small +for a call to realpath(). Use PATH_MAX if it's available and larger +than _POSIX_PATH_MAX, in a way that should be safe even when PATH_MAX +is not defined. + +Change-Id: I39127e88d92b358245ece21131219380ca4be98a +Reviewed-on: http://gerrit.openafs.org/11453 +Tested-by: BuildBot +Reviewed-by: Jeffrey Altman +Reviewed-by: Chas Williams - CONTRACTOR +Reviewed-by: Perry Ruiter +Reviewed-by: D Brashear +Tested-by: D Brashear +(cherry picked from commit ec2382e060753dfdcaf84b9ac03e1534c65fcdbc) +--- + src/util/dirpath.hin | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/src/util/dirpath.hin b/src/util/dirpath.hin +index 2890426..ae6d2cf 100644 +--- a/src/util/dirpath.hin ++++ b/src/util/dirpath.hin +@@ -75,7 +75,14 @@ + #ifdef AFS_NT40_ENV + #define AFSDIR_PATH_MAX MAX_PATH + #else /* unices */ +-#define AFSDIR_PATH_MAX _POSIX_PATH_MAX ++# ifndef PATH_MAX ++# define PATH_MAX 1024 ++# endif ++# if PATH_MAX > _POSIX_PATH_MAX ++# define AFSDIR_PATH_MAX PATH_MAX ++# else ++# define AFSDIR_PATH_MAX _POSIX_PATH_MAX ++# endif + #endif + + diff --git a/debian/patches/series b/debian/patches/series new file mode 100644 index 000000000..5d00dbe02 --- /dev/null +++ b/debian/patches/series @@ -0,0 +1,2 @@ +0001-userok.c-Fix-fixed-size-on-stack-path-buffers.patch +0002-Tweak-AFSDIR_PATH_MAX-definition.patch -- 2.39.5