I reviewed backuppc-rsync 3.1.3.0-3 as checked into hirsute. This shouldn't be considered a full audit but rather a quick gauge of maintainability. backuppc-rsync is a purpose-specific fork of rsync that is used by the backuppc tool, to use their format for storing backups. - CVE History: None in our database - Build-Depends? debhelper-compat (= 13), libacl1-dev, libattr1-dev, libpopt-dev, zlib1g-dev, - pre/post inst/rm scripts? None - init scripts? None - systemd units? None - dbus services? None - setuid binaries? None - binaries in PATH? None - sudo fragments? None - polkit files? None - udev rules? None - unit tests / autopkgtests? Pretty sure None - cron jobs? None - Build logs: Some very-old-code kinds of warnings, not great, but probably not very useful in the threat model - Processes spawned? Uses arrays - Memory management? there's way too much old-school C stuff - File IO? Quite a lot of it; controlled by remote execution. - Logging? Looked fine - Environment variable usage? Looked fine - Use of privileged functions? Not much, looked fine - Use of cryptography / random number sources etc? None - Use of temp files? None - Use of networking? Some, looked fine - Use of WebKit? None - Use of PolicyKit? None - Any significant cppcheck results? Nothing too bad in the context of this tool - Any significant Coverity results? Nothing too bad, upstream handled the issues quickly - Any significant shellcheck results? None - Any significant bandit results? None backuppc-rsync is based on some *old* code, which is both good and bad. It's a time-worn tool with some coding practices that aren't fantastic today, in a language I wish we'd see less of, but being built on rsync is no bad thing. Upstream for backuppc-rsync was responsive, reached out for more information for the issues I filed, and handled them well. None of the issues were particularly pressing for the threat model here: it's not like the remote endpoint is going to be run under control of malicious entities. Security team ACK for promoting backuppc-rsync to main. Some notes I took while reviewing: bpc_poolRefFileRead() fd leak in multiple error returns read_delay_line() read() doesn't nul terminate deldelay_buf, but strchr() expects it to be nul-terminated bpc_attrib_dirRead() can pass fd to functions without being initialized? hard to untangle send_implied_dirs() buffer overwrite? hard to untangle -- 17999 send_extra_file_list() buffer overread? hard to untangle -- 17979 add_dirs_to_tree() buffer overwrite? hard to untangle, -- 17962 bpc_opendir() use-after-free, free(d->entries) bpc_attrib_dirRead() can use nRead without initializing it first bpc_attrib_xattrCopy() can leak 'key' if 'value' couldn't be allocated add_dirs_to_tree() buffer overwrite? hard to untangle -- 17930 send_extra_file_list() buffer overread? hard to untangle -- 17894 bpc_poolWrite_copyToPool() appears to read and write eight bytes at a time; is this a large operation? If this is responsible for a lot of IO this could be pretty brutally bad. bpc_poolWrite_copyToPool() can leak fdWrite if fdRead open() fails add_cnk_list_entry() may have integer overflow problems in malloc() and realloc() calls, I don't see any checks to prevent integer overflow. do_cmd() constructs a command to start on a remote host; I don't see anything wrong with it, but it's complicated. create_pid_file() creates the pid file mode 0666, assuming umask allows it. Probably best to not rely on umask for this. https://github.com/backuppc/rsync-bpc/issues/23 https://github.com/backuppc/rsync-bpc/issues/24