lib/model: Stricter temporary file permissions

We could have a file to sync with permissions rw------- but we'd create
the temp file with rw-rw-rw- minus umask, usually rw-r--r--. This
potentially exposes private data while the file is being synced.

Similarly, when ignorePerms was set and we were reusing a temp files we
would set the permissions to rw-r--r-- explicitly, potentially
overriding a strict umask that would otherwise have had the file be
rw-------.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3437
This commit is contained in:
Jakob Borg 2016-07-25 10:18:05 +00:00 committed by Audrius Butkevicius
parent 6715b91a6c
commit 6ed22d0885
1 changed files with 20 additions and 4 deletions

View File

@ -121,24 +121,40 @@ func (s *sharedPullerState) tempFile() (io.WriterAt, error) {
} }
} }
// The permissions to use for the temporary file should be those of the
// final file, except we need user read & write at minimum. The
// permissions will be set to the final value later, but in the meantime
// we don't want to have a temporary file with looser permissions than
// the final outcome.
mode := os.FileMode(s.file.Permissions) | 0600
if s.ignorePerms {
// When ignorePerms is set we use a very permissive mode and let the
// system umask filter it.
mode = 0666
}
// Attempt to create the temp file // Attempt to create the temp file
// RDWR because of issue #2994. // RDWR because of issue #2994.
flags := os.O_RDWR flags := os.O_RDWR
if s.reused == 0 { if s.reused == 0 {
flags |= os.O_CREATE | os.O_EXCL flags |= os.O_CREATE | os.O_EXCL
} else { } else if !s.ignorePerms {
// With sufficiently bad luck when exiting or crashing, we may have // With sufficiently bad luck when exiting or crashing, we may have
// had time to chmod the temp file to read only state but not yet // had time to chmod the temp file to read only state but not yet
// moved it to it's final name. This leaves us with a read only temp // moved it to it's final name. This leaves us with a read only temp
// file that we're going to try to reuse. To handle that, we need to // file that we're going to try to reuse. To handle that, we need to
// make sure we have write permissions on the file before opening it. // make sure we have write permissions on the file before opening it.
err := os.Chmod(s.tempName, 0644) //
if !s.ignorePerms && err != nil { // When ignorePerms is set we trust that the permissions are fine
// already and make no modification, as we would otherwise override
// what the umask dictates.
if err := os.Chmod(s.tempName, mode); err != nil {
s.failLocked("dst create chmod", err) s.failLocked("dst create chmod", err)
return nil, err return nil, err
} }
} }
fd, err := os.OpenFile(s.tempName, flags, 0666) fd, err := os.OpenFile(s.tempName, flags, mode)
if err != nil { if err != nil {
s.failLocked("dst create", err) s.failLocked("dst create", err)
return nil, err return nil, err