fix: #258 normalize mode and truncate on --inplace overwrite
The --inplace branch opened the destination with O_WRONLY|O_CREAT (no O_TRUNC) and only restored metadata when the sender supplied it. Two flaws resulted: 1. An existing destination file kept its original mode when no metadata was sent, so setuid/setgid/sticky bits survived an overwrite (a root sync could leave a root-owned setuid binary controlled by a client). 2. A shorter payload left stale trailing bytes from the previous version because the file was never truncated to the new length. In the inplace branch of file_to_disk_secure_impl: - Always trim the file to the new payload length (ftruncate after the write) so stale trailing bytes can never survive; sparse targets keep their pre-size ftruncate. - Always normalize the mode after a successful overwrite: apply the metadata-derived safe mode when metadata is present (as before), else fchmod to a safe default 0644, so setuid/setgid/sticky are cleared in both cases. - The --update newer-destination check still runs before any truncation or chmod, preserving the skip semantics. Adds unit tests in test_file.c: (a) setuid/sticky bits on an existing destination are cleared after an inplace write with and without metadata, (b) a shorter inplace payload leaves no trailing stale bytes.
This commit is contained in:
+18
-3
@@ -348,10 +348,25 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
if (newer) {
|
||||
ok = true;
|
||||
} else {
|
||||
if (!sparse || data_size == 0 || ftruncate(fd, (off_t)data_size) == 0)
|
||||
/* In-place overwrites: pre-size sparse targets and always trim the
|
||||
file to the new payload length afterwards so shorter payloads can
|
||||
never leave stale trailing bytes from a previous version. */
|
||||
if (sparse && data_size > 0)
|
||||
ok = ftruncate(fd, (off_t)data_size) == 0;
|
||||
if (ok || !sparse || data_size == 0)
|
||||
ok = write_all(fd, data, data_size);
|
||||
if (ok && metadata)
|
||||
ok = file_restore_metadata_fd(fd, metadata, preserve_executability);
|
||||
if (ok)
|
||||
ok = ftruncate(fd, (off_t)data_size) == 0;
|
||||
/* Normalize the mode: apply the metadata-derived safe mode when the
|
||||
sender supplied metadata (setuid/setgid/sticky are never honored);
|
||||
otherwise fall back to a safe default so dangerous bits on an
|
||||
existing destination cannot survive an overwrite. */
|
||||
if (ok) {
|
||||
if (metadata)
|
||||
ok = file_restore_metadata_fd(fd, metadata, preserve_executability);
|
||||
else if (fchmod(fd, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) != 0)
|
||||
ok = false;
|
||||
}
|
||||
if (ok && use_fsync)
|
||||
ok = fsync(fd) == 0;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user