Skip to content

Fix CLI Handling of Permissions and Ownership (Again) #3432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 21, 2023
Merged

Fix CLI Handling of Permissions and Ownership (Again) #3432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2ad6855
`FIO_openSrcFile()` Returns File Stat
felixhandte Jan 17, 2023
5653f96
Pass `stat_t` into `*_dstFile()` Functions
felixhandte Jan 17, 2023
a5ed28f
Use Existing Src File Stat in `*_dstFile()` Funcs
felixhandte Jan 17, 2023
0382076
Re-Use `stat_t` in `FIO_compressFilename_srcFile()`
felixhandte Jan 17, 2023
1e3eba6
Copy Permissions from Source File
felixhandte Jan 17, 2023
0d2d460
Mimic gzip chown(gid), chmod(), chown(uid) Behavior
felixhandte Jan 18, 2023
7a8c8f3
Easy: Print Mode as Octal in `chmod()` Trace
felixhandte Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 65 additions & 47 deletions programs/fileio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,15 @@ char const* FIO_lzmaVersion(void)
#define FNSPACE 30

/* Default file permissions 0666 (modulated by umask) */
/* Temporary restricted file permissions are used when we're going to
* chmod/chown at the end of the operation. */
#if !defined(_WIN32)
/* These macros aren't defined on windows. */
#define DEFAULT_FILE_PERMISSIONS (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)
#define TEMPORARY_FILE_PERMISSIONS (S_IRUSR|S_IWUSR)
#else
#define DEFAULT_FILE_PERMISSIONS (0666)
#define TEMPORARY_FILE_PERMISSIONS (0600)
#endif

/*-************************************
Expand Down Expand Up @@ -531,26 +535,26 @@ static int FIO_removeFile(const char* path)
/** FIO_openSrcFile() :
* condition : `srcFileName` must be non-NULL. `prefs` may be NULL.
* @result : FILE* to `srcFileName`, or NULL if it fails */
static FILE* FIO_openSrcFile(const FIO_prefs_t* const prefs, const char* srcFileName)
static FILE* FIO_openSrcFile(const FIO_prefs_t* const prefs, const char* srcFileName, stat_t* statbuf)
{
stat_t statbuf;
int allowBlockDevices = prefs != NULL ? prefs->allowBlockDevices : 0;
assert(srcFileName != NULL);
assert(statbuf != NULL);
if (!strcmp (srcFileName, stdinmark)) {
DISPLAYLEVEL(4,"Using stdin for input \n");
SET_BINARY_MODE(stdin);
return stdin;
}

if (!UTIL_stat(srcFileName, &statbuf)) {
if (!UTIL_stat(srcFileName, statbuf)) {
DISPLAYLEVEL(1, "zstd: can't stat %s : %s -- ignored \n",
srcFileName, strerror(errno));
return NULL;
}

if (!UTIL_isRegularFileStat(&statbuf)
&& !UTIL_isFIFOStat(&statbuf)
&& !(allowBlockDevices && UTIL_isBlockDevStat(&statbuf))
if (!UTIL_isRegularFileStat(statbuf)
&& !UTIL_isFIFOStat(statbuf)
&& !(allowBlockDevices && UTIL_isBlockDevStat(statbuf))
) {
DISPLAYLEVEL(1, "zstd: %s is not a regular file -- ignored \n",
srcFileName);
Expand Down Expand Up @@ -662,23 +666,23 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs,
* @return : loaded size
* if fileName==NULL, returns 0 and a NULL pointer
*/
static size_t FIO_createDictBuffer(void** bufferPtr, const char* fileName, FIO_prefs_t* const prefs)
static size_t FIO_createDictBuffer(void** bufferPtr, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
{
FILE* fileHandle;
U64 fileSize;
stat_t statbuf;

assert(bufferPtr != NULL);
assert(dictFileStat != NULL);
*bufferPtr = NULL;
if (fileName == NULL) return 0;

DISPLAYLEVEL(4,"Loading %s as dictionary \n", fileName);

if (!UTIL_stat(fileName, &statbuf)) {
if (!UTIL_stat(fileName, dictFileStat)) {
EXM_THROW(31, "Stat failed on dictionary file %s: %s", fileName, strerror(errno));
}

if (!UTIL_isRegularFileStat(&statbuf)) {
if (!UTIL_isRegularFileStat(dictFileStat)) {
EXM_THROW(32, "Dictionary %s must be a regular file.", fileName);
}

Expand All @@ -688,7 +692,7 @@ static size_t FIO_createDictBuffer(void** bufferPtr, const char* fileName, FIO_p
EXM_THROW(33, "Couldn't open dictionary %s: %s", fileName, strerror(errno));
}

fileSize = UTIL_getFileSizeStat(&statbuf);
fileSize = UTIL_getFileSizeStat(dictFileStat);
{
size_t const dictSizeMax = prefs->patchFromMode ? prefs->memLimit : DICTSIZE_MAX;
if (fileSize > dictSizeMax) {
Expand Down Expand Up @@ -869,6 +873,7 @@ typedef struct {
void* dictBuffer;
size_t dictBufferSize;
const char* dictFileName;
stat_t dictFileStat;
ZSTD_CStream* cctx;
WritePoolCtx_t *writeCtx;
ReadPoolCtx_t *readCtx;
Expand Down Expand Up @@ -927,7 +932,7 @@ static cRess_t FIO_createCResources(FIO_prefs_t* const prefs,
unsigned long long const ssSize = (unsigned long long)prefs->streamSrcSize;
FIO_adjustParamsForPatchFromMode(prefs, &comprParams, UTIL_getFileSize(dictFileName), ssSize > 0 ? ssSize : maxSrcFileSize, cLevel);
}
ress.dictBufferSize = FIO_createDictBuffer(&ress.dictBuffer, dictFileName, prefs); /* works with dictFileName==NULL */
ress.dictBufferSize = FIO_createDictBuffer(&ress.dictBuffer, dictFileName, prefs, &ress.dictFileStat); /* works with dictFileName==NULL */

ress.writeCtx = AIO_WritePool_create(prefs, ZSTD_CStreamOutSize());
ress.readCtx = AIO_ReadPool_create(prefs, ZSTD_CStreamInSize());
Expand Down Expand Up @@ -1626,27 +1631,27 @@ static int FIO_compressFilename_dstFile(FIO_ctx_t* const fCtx,
cRess_t ress,
const char* dstFileName,
const char* srcFileName,
const stat_t* srcFileStat,
int compressionLevel)
{
int closeDstFile = 0;
int result;
stat_t statbuf;
int transferMTime = 0;
int transferStat = 0;
FILE *dstFile;

assert(AIO_ReadPool_getFile(ress.readCtx) != NULL);
if (AIO_WritePool_getFile(ress.writeCtx) == NULL) {
int dstFilePermissions = DEFAULT_FILE_PERMISSIONS;
int dstFileInitialPermissions = DEFAULT_FILE_PERMISSIONS;
if ( strcmp (srcFileName, stdinmark)
&& strcmp (dstFileName, stdoutmark)
&& UTIL_stat(srcFileName, &statbuf)
&& UTIL_isRegularFileStat(&statbuf) ) {
dstFilePermissions = statbuf.st_mode;
transferMTime = 1;
&& UTIL_isRegularFileStat(srcFileStat) ) {
transferStat = 1;
dstFileInitialPermissions = TEMPORARY_FILE_PERMISSIONS;
}

closeDstFile = 1;
DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: opening dst: %s \n", dstFileName);
dstFile = FIO_openDstFile(fCtx, prefs, srcFileName, dstFileName, dstFilePermissions);
dstFile = FIO_openDstFile(fCtx, prefs, srcFileName, dstFileName, dstFileInitialPermissions);
if (dstFile==NULL) return 1; /* could not open dstFileName */
AIO_WritePool_setFile(ress.writeCtx, dstFile);
/* Must only be added after FIO_openDstFile() succeeds.
Expand All @@ -1666,8 +1671,8 @@ static int FIO_compressFilename_dstFile(FIO_ctx_t* const fCtx,
DISPLAYLEVEL(1, "zstd: %s: %s \n", dstFileName, strerror(errno));
result=1;
}
if (transferMTime) {
UTIL_utime(dstFileName, &statbuf);
if (transferStat) {
UTIL_setFileStat(dstFileName, srcFileStat);
}
if ( (result != 0) /* operation failure */
&& strcmp(dstFileName, stdoutmark) /* special case : don't remove() stdout */
Expand Down Expand Up @@ -1709,18 +1714,25 @@ FIO_compressFilename_srcFile(FIO_ctx_t* const fCtx,
{
int result;
FILE* srcFile;
stat_t srcFileStat;
DISPLAYLEVEL(6, "FIO_compressFilename_srcFile: %s \n", srcFileName);

/* ensure src is not a directory */
if (UTIL_isDirectory(srcFileName)) {
DISPLAYLEVEL(1, "zstd: %s is a directory -- ignored \n", srcFileName);
return 1;
}
if (strcmp(srcFileName, stdinmark)) {
if (UTIL_stat(srcFileName, &srcFileStat)) {
/* failure to stat at all is handled during opening */

/* ensure src is not the same as dict (if present) */
if (ress.dictFileName != NULL && UTIL_isSameFile(srcFileName, ress.dictFileName)) {
DISPLAYLEVEL(1, "zstd: cannot use %s as an input file and dictionary \n", srcFileName);
return 1;
/* ensure src is not a directory */
if (UTIL_isDirectoryStat(&srcFileStat)) {
DISPLAYLEVEL(1, "zstd: %s is a directory -- ignored \n", srcFileName);
return 1;
}

/* ensure src is not the same as dict (if present) */
if (ress.dictFileName != NULL && UTIL_isSameFileStat(srcFileName, ress.dictFileName, &srcFileStat, &ress.dictFileStat)) {
DISPLAYLEVEL(1, "zstd: cannot use %s as an input file and dictionary \n", srcFileName);
return 1;
}
}
}

/* Check if "srcFile" is compressed. Only done if --exclude-compressed flag is used
Expand All @@ -1732,11 +1744,14 @@ FIO_compressFilename_srcFile(FIO_ctx_t* const fCtx,
return 0;
}

srcFile = FIO_openSrcFile(prefs, srcFileName);
srcFile = FIO_openSrcFile(prefs, srcFileName, &srcFileStat);
if (srcFile == NULL) return 1; /* srcFile could not be opened */

AIO_ReadPool_setFile(ress.readCtx, srcFile);
result = FIO_compressFilename_dstFile(fCtx, prefs, ress, dstFileName, srcFileName, compressionLevel);
result = FIO_compressFilename_dstFile(
fCtx, prefs, ress,
dstFileName, srcFileName,
&srcFileStat, compressionLevel);
AIO_ReadPool_closeFile(ress.readCtx);

if ( prefs->removeSrcFile /* --rm */
Expand Down Expand Up @@ -1994,7 +2009,8 @@ static dRess_t FIO_createDResources(FIO_prefs_t* const prefs, const char* dictFi

/* dictionary */
{ void* dictBuffer;
size_t const dictBufferSize = FIO_createDictBuffer(&dictBuffer, dictFileName, prefs);
stat_t statbuf;
size_t const dictBufferSize = FIO_createDictBuffer(&dictBuffer, dictFileName, prefs, &statbuf);
CHECK( ZSTD_DCtx_reset(ress.dctx, ZSTD_reset_session_only) );
CHECK( ZSTD_DCtx_loadDictionary(ress.dctx, dictBuffer, dictBufferSize) );
free(dictBuffer);
Expand Down Expand Up @@ -2461,22 +2477,22 @@ static int FIO_decompressFrames(FIO_ctx_t* const fCtx,
static int FIO_decompressDstFile(FIO_ctx_t* const fCtx,
FIO_prefs_t* const prefs,
dRess_t ress,
const char* dstFileName, const char* srcFileName)
const char* dstFileName,
const char* srcFileName,
const stat_t* srcFileStat)
{
int result;
stat_t statbuf;
int releaseDstFile = 0;
int transferMTime = 0;
int transferStat = 0;

if ((AIO_WritePool_getFile(ress.writeCtx) == NULL) && (prefs->testMode == 0)) {
FILE *dstFile;
int dstFilePermissions = DEFAULT_FILE_PERMISSIONS;
if ( strcmp(srcFileName, stdinmark) /* special case : don't transfer permissions from stdin */
&& strcmp(dstFileName, stdoutmark)
&& UTIL_stat(srcFileName, &statbuf)
&& UTIL_isRegularFileStat(&statbuf) ) {
dstFilePermissions = statbuf.st_mode;
transferMTime = 1;
&& UTIL_isRegularFileStat(srcFileStat) ) {
transferStat = 1;
dstFilePermissions = TEMPORARY_FILE_PERMISSIONS;
}

releaseDstFile = 1;
Expand All @@ -2501,8 +2517,8 @@ static int FIO_decompressDstFile(FIO_ctx_t* const fCtx,
result = 1;
}

if (transferMTime) {
UTIL_utime(dstFileName, &statbuf);
if (transferStat) {
UTIL_setFileStat(dstFileName, srcFileStat);
}

if ( (result != 0) /* operation failure */
Expand All @@ -2524,18 +2540,19 @@ static int FIO_decompressDstFile(FIO_ctx_t* const fCtx,
static int FIO_decompressSrcFile(FIO_ctx_t* const fCtx, FIO_prefs_t* const prefs, dRess_t ress, const char* dstFileName, const char* srcFileName)
{
FILE* srcFile;
stat_t srcFileStat;
int result;

if (UTIL_isDirectory(srcFileName)) {
DISPLAYLEVEL(1, "zstd: %s is a directory -- ignored \n", srcFileName);
return 1;
}

srcFile = FIO_openSrcFile(prefs, srcFileName);
srcFile = FIO_openSrcFile(prefs, srcFileName, &srcFileStat);
if (srcFile==NULL) return 1;
AIO_ReadPool_setFile(ress.readCtx, srcFile);

result = FIO_decompressDstFile(fCtx, prefs, ress, dstFileName, srcFileName);
result = FIO_decompressDstFile(fCtx, prefs, ress, dstFileName, srcFileName, &srcFileStat);

AIO_ReadPool_setFile(ress.readCtx, NULL);

Expand Down Expand Up @@ -2899,10 +2916,11 @@ static InfoError
getFileInfo_fileConfirmed(fileInfo_t* info, const char* inFileName)
{
InfoError status;
FILE* const srcFile = FIO_openSrcFile(NULL, inFileName);
stat_t srcFileStat;
FILE* const srcFile = FIO_openSrcFile(NULL, inFileName, &srcFileStat);
ERROR_IF(srcFile == NULL, info_file_error, "Error: could not open source file %s", inFileName);

info->compressedSize = UTIL_getFileSize(inFileName);
info->compressedSize = UTIL_getFileSizeStat(&srcFileStat);
status = FIO_analyzeFrames(info, srcFile);

fclose(srcFile);
Expand Down
54 changes: 47 additions & 7 deletions programs/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ int UTIL_isRegularFileStat(const stat_t* statbuf)
int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions)
{
stat_t localStatBuf;
UTIL_TRACE_CALL("UTIL_chmod(%s, %u)", filename, (unsigned)permissions);
UTIL_TRACE_CALL("UTIL_chmod(%s, %#4o)", filename, (unsigned)permissions);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First time I see this formating instruction...

if (statbuf == NULL) {
if (!UTIL_stat(filename, &localStatBuf)) {
UTIL_TRACE_RET(0);
Expand Down Expand Up @@ -249,11 +249,23 @@ int UTIL_setFileStat(const char *filename, const stat_t *statbuf)
/* set access and modification times */
res += UTIL_utime(filename, statbuf);

/* Mimic gzip's behavior:
*
* "Change the group first, then the permissions, then the owner.
* That way, the permissions will be correct on systems that allow
* users to give away files, without introducing a security hole.
* Security depends on permissions not containing the setuid or
* setgid bits." */

#if !defined(_WIN32)
res += chown(filename, statbuf->st_uid, statbuf->st_gid); /* Copy ownership */
res += chown(filename, -1, statbuf->st_gid); /* Apply group ownership */
#endif

res += UTIL_chmod(filename, &curStatBuf, statbuf->st_mode & 07777); /* Copy file permissions */
res += UTIL_chmod(filename, &curStatBuf, statbuf->st_mode & 0777); /* Copy file permissions */

#if !defined(_WIN32)
res += chown(filename, statbuf->st_uid, -1); /* Apply user ownership */
#endif

errno = 0;
UTIL_TRACE_RET(-res);
Expand All @@ -272,11 +284,15 @@ int UTIL_isDirectory(const char* infilename)

int UTIL_isDirectoryStat(const stat_t* statbuf)
{
int ret;
UTIL_TRACE_CALL("UTIL_isDirectoryStat()");
#if defined(_MSC_VER)
return (statbuf->st_mode & _S_IFDIR) != 0;
ret = (statbuf->st_mode & _S_IFDIR) != 0;
#else
return S_ISDIR(statbuf->st_mode) != 0;
ret = S_ISDIR(statbuf->st_mode) != 0;
#endif
UTIL_TRACE_RET(ret);
return ret;
}

int UTIL_compareStr(const void *p1, const void *p2) {
Expand All @@ -299,8 +315,32 @@ int UTIL_isSameFile(const char* fName1, const char* fName2)
stat_t file2Stat;
ret = UTIL_stat(fName1, &file1Stat)
&& UTIL_stat(fName2, &file2Stat)
&& (file1Stat.st_dev == file2Stat.st_dev)
&& (file1Stat.st_ino == file2Stat.st_ino);
&& UTIL_isSameFileStat(fName1, fName2, &file1Stat, &file2Stat);
}
#endif
UTIL_TRACE_RET(ret);
return ret;
}

int UTIL_isSameFileStat(
const char* fName1, const char* fName2,
const stat_t* file1Stat, const stat_t* file2Stat)
{
int ret;
assert(fName1 != NULL); assert(fName2 != NULL);
UTIL_TRACE_CALL("UTIL_isSameFileStat(%s, %s)", fName1, fName2);
#if defined(_MSC_VER) || defined(_WIN32)
/* note : Visual does not support file identification by inode.
* inode does not work on Windows, even with a posix layer, like msys2.
* The following work-around is limited to detecting exact name repetition only,
* aka `filename` is considered different from `subdir/../filename` */
(void)file1Stat;
(void)file2Stat;
ret = !strcmp(fName1, fName2);
#else
{
ret = (file1Stat->st_dev == file2Stat->st_dev)
&& (file1Stat->st_ino == file2Stat->st_ino);
}
#endif
UTIL_TRACE_RET(ret);
Expand Down
1 change: 1 addition & 0 deletions programs/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions);
int UTIL_isRegularFile(const char* infilename);
int UTIL_isDirectory(const char* infilename);
int UTIL_isSameFile(const char* file1, const char* file2);
int UTIL_isSameFileStat(const char* file1, const char* file2, const stat_t* file1Stat, const stat_t* file2Stat);
int UTIL_isCompressedFile(const char* infilename, const char *extensionList[]);
int UTIL_isLink(const char* infilename);
int UTIL_isFIFO(const char* infilename);
Expand Down
1 change: 1 addition & 0 deletions tests/cli-tests/file-stat/compress-file-to-file.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
set -e

datagen > file
chmod 642 file

zstd file -q --trace-file-stat -o file.zst
zstd -tq file.zst
Loading