From: W. Felix Handte Date: Mon, 1 Mar 2021 17:23:52 +0000 (+0000) Subject: fix race condition allowing attackers to access destination file X-Git-Tag: archive/raspbian/1.4.8+dfsg-3+rpi1~1^2~1 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=497f26598f1e8a7b2b487ff2ff598ddcb5c5b743;p=libzstd.git fix race condition allowing attackers to access destination file Origin: upstream Bug: https://github.com/facebook/zstd/issues/2491 Bug-Debian: https://github.com/facebook/zstd/issues/2491 Applied-Upstream: commit:a774c5797399040af62db21d8a9b9769e005430e Reviewed-by: Étienne Mollier Last-Update: 2021-02-18 This commit addresses https://github.com/facebook/zstd/issues/2491. Note that a downside of this solution is that it is global: `umask()` affects all file creation calls in the process. I believe this is safe since `fileio.c` functions should only ever be used in the zstd binary, and these are (almost) the only files ever created by zstd, and AIUI they're only created in a single thread. So we can get away with messing with global state. Note that this doesn't change the permissions of files created by `dibio.c`. I'm not sure what those should be... Last-Update: 2021-02-18 Gbp-Pq: Name 0018-fix-file-permissions-on-compression.patch --- diff --git a/programs/fileio.c b/programs/fileio.c index 65f2d53..e740876 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -675,14 +675,11 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, FIO_removeFile(dstFileName); } } - { FILE* const f = fopen( dstFileName, "wb" ); + { const int old_umask = UTIL_umask(0177); /* u-x,go-rwx */ + FILE* const f = fopen( dstFileName, "wb" ); + UTIL_umask(old_umask); if (f == NULL) { DISPLAYLEVEL(1, "zstd: %s: %s\n", dstFileName, strerror(errno)); - } else if (srcFileName != NULL - && strcmp (srcFileName, stdinmark) - && strcmp(dstFileName, nulmark) ) { - /* reduce rights on newly created dst file while compression is ongoing */ - UTIL_chmod(dstFileName, NULL, 00600); } return f; } diff --git a/programs/util.c b/programs/util.c index 8bcfe47..6f7810c 100644 --- a/programs/util.c +++ b/programs/util.c @@ -159,6 +159,15 @@ int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions) return chmod(filename, permissions); } +int UTIL_umask(int mode) { +#if PLATFORM_POSIX_VERSION > 0 + return umask(mode); +#else + /* do nothing, fake return value */ + return mode; +#endif +} + int UTIL_setFileStat(const char *filename, const stat_t *statbuf) { int res = 0; diff --git a/programs/util.h b/programs/util.h index 25fa3f5..2c31a2d 100644 --- a/programs/util.h +++ b/programs/util.h @@ -22,7 +22,7 @@ extern "C" { #include "platform.h" /* PLATFORM_POSIX_VERSION, ZSTD_NANOSLEEP_SUPPORT, ZSTD_SETPRIORITY_SUPPORT */ #include /* size_t, ptrdiff_t */ #include /* stat, utime */ -#include /* stat, chmod */ +#include /* stat, chmod, umask */ #include "../lib/common/mem.h" /* U64 */ @@ -152,6 +152,11 @@ U64 UTIL_getFileSizeStat(const stat_t* statbuf); */ int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions); +/** + * Wraps umask(). Does nothing when the platform doesn't have that concept. + */ +int UTIL_umask(int mode); + /* * In the absence of a pre-existing stat result on the file in question, these * functions will do a stat() call internally and then use that result to