[PATCH] sftp: don't clobber permissions of an existing local directory on download

[Muhammad Bilal]

download_dir_internal() in sftp-client.c calls mkdir(dst, tmpmode) and
ignores EEXIST, but never records whether it actually created the
directory. At the end of the function it unconditionally does:

    if (mode != tmpmode && chmod(dst, mode) == -1)

where 'mode' is the remote directory's permissions. This runs even
when 'dst' already existed locally and -p/preserve was not requested,
so 'sftp get -r' into an existing local directory silently rewrites
that directory's mode to match the remote side.

upload_dir_internal() and crossload_dir_internal() already guard their
equivalent remote-side sftp_setstat() call with a 'created' flag added
for bz3925 ("don't clobber the remote directory permissions unless
either we created the directory during the transfer or the -p flag
was set"); download_dir_internal() was missed. This gives it the same
guard: only chmod(2) the local directory if this call created it, or
if -p/preserve_flag was explicitly requested.

Verified with a local loopback sftp/sftp-server session
(sftp -D ./sftp-server): before the fix, 'get -r' into a pre-existing
local directory whose mode didn't already include the owner
write/execute bits (so mode != tmpmode) rewrote that directory's mode
to the remote's; after the fix it is left alone unless -p is given or
the directory was newly created, matching upload's behaviour.
---
 sftp-client.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/sftp-client.c b/sftp-client.c
index 1f031128e..74dd434ca 100644
--- a/sftp-client.c
+++ b/sftp-client.c
@@ -1882,7 +1882,7 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
     int depth, Attrib *dirattrib, int preserve_flag, int print_flag,
     int resume_flag, int fsync_flag, int follow_link_flag, int inplace_flag)
 {
-	int i, ret = 0;
+	int i, ret = 0, created = 0;
 	SFTP_DIRENT **dir_entries;
 	char *filename, *new_src = NULL, *new_dst = NULL;
 	mode_t mode = 0777, tmpmode = mode;
@@ -1917,10 +1917,13 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
 		    "did not send permissions", dst);
 	}
 
-	if (mkdir(dst, tmpmode) == -1 && errno != EEXIST) {
-		error("mkdir %s: %s", dst, strerror(errno));
-		return -1;
-	}
+	if (mkdir(dst, tmpmode) == -1) {
+		if (errno != EEXIST) {
+			error("mkdir %s: %s", dst, strerror(errno));
+			return -1;
+		}
+	} else
+		created = 1;
 
 	if (sftp_readdir(conn, src, &dir_entries) == -1) {
 		error("remote readdir \"%s\" failed", src);
@@ -1989,7 +1992,8 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
 			    "\"%s\"", dst);
 	}
 
-	if (mode != tmpmode && chmod(dst, mode) == -1)
+	if ((created || preserve_flag) && mode != tmpmode &&
+	    chmod(dst, mode) == -1)
 		error("local chmod directory \"%s\": %s", dst,
 		    strerror(errno));
 
-- 
2.43.0