Description: fix race condition allowing attackers to access destination file 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... Author: W. Felix Handte 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-03-03 --- This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ --- a/programs/fileio.c +++ b/programs/fileio.c @@ -606,11 +606,11 @@ FIO_openDstFile(FIO_prefs_t* const prefs FIO_remove(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)) { - chmod(dstFileName, 00600); } return f; } --- a/programs/util.c +++ b/programs/util.c @@ -54,6 +54,15 @@ int UTIL_getFileStat(const char* infilen return 1; } +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, stat_t *statbuf) { int res = 0; --- a/programs/util.h +++ b/programs/util.h @@ -136,6 +136,10 @@ int UTIL_isSameFile(const char* file1, c int UTIL_compareStr(const void *p1, const void *p2); int UTIL_isCompressedFile(const char* infilename, const char *extensionList[]); const char* UTIL_getFileExtension(const char* infilename); +/** + * Wraps umask(). Does nothing when the platform doesn't have that concept. + */ +int UTIL_umask(int mode); #ifndef _MSC_VER U32 UTIL_isFIFO(const char* infilename);