When to call CheckCopiedFile() ?

[Sergio . Gelato]

Currently (2.1.0p1) cfengine copies regular files in CopyReg(), then
adjusts their ownership, mode bits, ACLs, etc in CheckCopiedFile()
which is little more than a wrapper around CheckExistingFile().

CopyReg() makes no attempt to set the new file's ownership and
permissions to their final values before rename()ing it into place.
As a result, there is a time window in which a newly copied file will
have the wrong permissions.

I propose to call CheckCopiedFile() from within CopyReg(), just before
the rename(new,dest) call.

There may be an impact on disk quota (since the new file's ownership is
set before the old version is deleted), but I think that's less of a
problem than allowing the file to have the wrong permissions.

I note in passing a couple of other issues in CopyReg() related to the
handling of the backup file:

1. potential buffer overflows in forming the name of the backup file,
   particularly when IMAGEBACKUP == 's'. Could be regarded as a
   documentation issue ("Don't use very long destination file names"---
   "Yes, but just how long is too long?"). But why doesn't cfengine
   use strlcat() (or at least strncat()) throughout?
2. rename(backup,dest) is attempted unconditionally, even when
   IMAGEBACKUP == 'n' (in which case the backup variable is undefined).
3. Wouldn't it be better to perform the sanity checks (size, checksum)
   on the new file first, and only back up/remove the old file if they
   succeed ?

Patches will follow shortly.