Currently we trust the server to not send any unsafe filenames. Think about the implications of this.

Attack surface

• All names used for temporary files are entirely client-determined.
• Server-specified filenames are only ever passed through standard utilities, passed to rsync with --files-from, or to sha1sum --check.
• One line from the file is extracted from the file list and used in a string comparison in the shell.
• Filenames are used on the command line when deleting files and directories, since rm does not take a list of files/dirs to remove. The set of files to be removed is determined solely by what is present on the local filesystem, but this is still indirectly controlled by the server because it could send arbitrary filenames in one run and then not list them in the next.

We must at some point trust the standard utilities, rsync and sha1sum. Assuming that, the server can do the following to the client:

• Send a corrupted file list not in a format parseable by the client, rsync or sha1sum.
• The client just calls awk but except for one case only passes the result to either rsync or sort.

• One line from the file is captured as input and used in the shell in a '[[' test. A modern shell should be safe against this, and the variable is quoted.

• Send a huge file list, causing excessive processing or disk usage on the client.

  • File lists (both from the server and the derived ones made by the client) are stored in wherever mktemp creates directory. If this is a tmpfs volume, it will consume memory and potentially fill /tmp. If this is a concern for an admin, they can specify TMPDIR in the client configuration since the config file is sourced before mktemp is called.
  • File lists are processed by standard utilities, and we have to assume it is safe to give untrusted data as input (not on the command line).
  • Since sort is used to compute an asymmetric set difference between two files, incredibly large files or those with ordering pathological to the sorting algorithm could consume more CPU or memory than necessary. The local administrator can run the script with limits in place if they are concerned about this.

• Send an empty file list, causing all files in the local repository to be considered stale and be deleted.

• Send a an arbitrary filename, and then have the client remove it.

• Simply send huge files, filling the disk which holds the repository.

• Send huge files and then remove them in two different runs, consuming arbitrary network bandwidth (though this consumes bandwidth on the server, too).

Concerns

If the server sends a filename like "; rm -rf /" and then on the next run doesn't specify that file, it will be marked as stale and deleted. The deletion process generates a set of filenames, zero terminates that (by converting newlines to NULs), and passes them to xargs -0 rm -f. This is not subject to shell expansion so the string is simply passed as another filename to delete.

In my testing with embedding semicolons and other shell metachars is not problematic.

If a local file/directory containing a newline is present, the rm won't delete it because of the conversion between newlines and NULs. It will simply see two files/dirs. If one of these is '..\n' then the client could be made to delete the directory above the repository, which will be within the master module. However, since the file list and rsync's --files-from list are newline separated, it is not possible for the server to send such a file to the client. (Note that rsync is not called with recursion enabled, so only files explicitly present in the file list will be transferred.)

Since the files to be removed are not specified directly by the server, it's not possible for the server to ask for the deletion of a file containing the directory separator.

Possible things to fix

• Add some limit to the quantity of files deleted per run. Not really useful unless we also don't delete directories.

• Add some additional checking for file list corruption (to avoid the server sending potentially missing filenames). Or simply deal with getting an empty filename. (Test what happens.)

• Implement setting of resource limits directly in the script via ulimit.