Commit | Line | Data |
---|---|---|
9feef62b LLB |
1 | Description: fix race condition allowing attackers to access destination file |
2 | This commit addresses https://github.com/facebook/zstd/issues/2491. | |
3 | . | |
4 | Note that a downside of this solution is that it is global: `umask()` affects | |
5 | all file creation calls in the process. I believe this is safe since | |
6 | `fileio.c` functions should only ever be used in the zstd binary, and these | |
7 | are (almost) the only files ever created by zstd, and AIUI they're only | |
8 | created in a single thread. So we can get away with messing with global state. | |
9 | . | |
10 | Note that this doesn't change the permissions of files created by `dibio.c`. | |
11 | I'm not sure what those should be... | |
12 | Author: W. Felix Handte <w@felixhandte.com> | |
13 | Origin: upstream | |
14 | Bug: https://github.com/facebook/zstd/issues/2491 | |
15 | Bug-Debian: https://github.com/facebook/zstd/issues/2491 | |
16 | Applied-Upstream: commit:a774c5797399040af62db21d8a9b9769e005430e | |
17 | Reviewed-by: Étienne Mollier <etienne.mollier@mailoo.org> | |
18 | Last-Update: 2021-03-03 | |
19 | --- | |
20 | This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ | |
21 | --- a/programs/fileio.c | |
22 | +++ b/programs/fileio.c | |
23 | @@ -606,11 +606,11 @@ FIO_openDstFile(FIO_prefs_t* const prefs | |
24 | FIO_remove(dstFileName); | |
25 | } } | |
26 | ||
27 | - { FILE* const f = fopen( dstFileName, "wb" ); | |
28 | + { const int old_umask = UTIL_umask(0177); /* u-x,go-rwx */ | |
29 | + FILE* const f = fopen( dstFileName, "wb" ); | |
30 | + UTIL_umask(old_umask); | |
31 | if (f == NULL) { | |
32 | DISPLAYLEVEL(1, "zstd: %s: %s\n", dstFileName, strerror(errno)); | |
33 | - } else if(srcFileName != NULL && strcmp (srcFileName, stdinmark)) { | |
34 | - chmod(dstFileName, 00600); | |
35 | } | |
36 | return f; | |
37 | } | |
38 | --- a/programs/util.c | |
39 | +++ b/programs/util.c | |
40 | @@ -54,6 +54,15 @@ int UTIL_getFileStat(const char* infilen | |
41 | return 1; | |
42 | } | |
43 | ||
44 | +int UTIL_umask(int mode) { | |
45 | +#if PLATFORM_POSIX_VERSION > 0 | |
46 | + return umask(mode); | |
47 | +#else | |
48 | + /* do nothing, fake return value */ | |
49 | + return mode; | |
50 | +#endif | |
51 | +} | |
52 | + | |
53 | int UTIL_setFileStat(const char *filename, stat_t *statbuf) | |
54 | { | |
55 | int res = 0; | |
56 | --- a/programs/util.h | |
57 | +++ b/programs/util.h | |
58 | @@ -136,6 +136,10 @@ int UTIL_isSameFile(const char* file1, c | |
59 | int UTIL_compareStr(const void *p1, const void *p2); | |
60 | int UTIL_isCompressedFile(const char* infilename, const char *extensionList[]); | |
61 | const char* UTIL_getFileExtension(const char* infilename); | |
62 | +/** | |
63 | + * Wraps umask(). Does nothing when the platform doesn't have that concept. | |
64 | + */ | |
65 | +int UTIL_umask(int mode); | |
66 | ||
67 | #ifndef _MSC_VER | |
68 | U32 UTIL_isFIFO(const char* infilename); |