From: W. Felix Handte <w@felixhandte.com>
Date: Thu, 18 Feb 2021 11:59:48 +0000 (+0000)
Subject: fix race condition allowing attackers to access destination file
X-Git-Tag: archive/raspbian/1.3.8+dfsg-3+rpi1+deb10u2^2~1
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=878957dca87f7aafc6144ec663a09513da487834;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://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982519
Applied-Upstream: commit:a774c5797399040af62db21d8a9b9769e005430e
Reviewed-by: Étienne Mollier <etienne.mollier@mailoo.org>
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 0017-fix-file-permissions-on-compression.patch
---

diff --git a/programs/fileio.c b/programs/fileio.c
index 9fb795e..e17a0b4 100644
--- a/programs/fileio.c
+++ b/programs/fileio.c
@@ -481,7 +481,9 @@ static FILE* FIO_openDstFile(const char* srcFileName, const char* dstFileName)
             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));
         return f;
diff --git a/programs/util.c b/programs/util.c
index 3463431..120ad40 100644
--- a/programs/util.c
+++ b/programs/util.c
@@ -51,6 +51,15 @@ int UTIL_getFileStat(const char* infilename, stat_t *statbuf)
     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;
diff --git a/programs/util.h b/programs/util.h
index f78bcbe..22a688e 100644
--- a/programs/util.h
+++ b/programs/util.h
@@ -181,6 +181,11 @@ U64 UTIL_getFileSize(const char* infilename);
 
 U64 UTIL_getTotalFileSize(const char* const * const fileNamesTable, unsigned nbFiles);
 
+/**
+ * Wraps umask(). Does nothing when the platform doesn't have that concept.
+ */
+int UTIL_umask(int mode);
+
 /*
  * A modified version of realloc().
  * If UTIL_realloc() fails the original block is freed.