cytopia / linux-timemachine

Rsync-based OSX-like time machine for Linux, MacOS and BSD for atomic and resumable local and remote backups
MIT License
780 stars 63 forks source link

Security update #65

Closed dmandala closed 3 years ago

dmandala commented 3 years ago

None of the applications were hard pathed, this could allow an attacker the ability to swap in a different binary by putting a different binary with the same name in the path so it's found before the correct binary or the attacker could change the path so the correct binary could not be found. With hard paths that is not possible to do, the attacker would have to modify the the actual binaries on the file system.

All binary's are now hard pathed so $PATH manipulation can not effect the running of the application. I did test the changes in my use, I did not use your tests so you may want to test further.

cytopia commented 3 years ago

I had a look into this and it seems that BSD, MacOS and Linux have certain binaries at different paths. Eventually also different versions/flavours of the OS might be different as well.

I was successfully testing the path manipulation:

$ PATH= ./timemachine
./timemachine: 118: ./timemachine: date: not found
timemachine: [ERROR] <source> and <destination> are required. See -h for help.

However, what can be done instead to ensure the path cannot be altered from outside, is to explicitly set it inside the timemachine script.

#!/bin/sh -eu
#
# https://serverfault.com/questions/834994/rsync-only-keep-10-backup-folders
export PATH=/bin:/usr/bin
...

The exported $PATH will only be valid/scoped within the script itself and will not effect the outside environment. So with this set, I've tried the path manipulation again:

$ PATH= ./timemachine
2021-04-01 12:26:35 timemachine: [ERROR] <source> and <destination> are required. See -h for help.

So this looks like it is fixing the issue of overriding the path from outside.

Note: In the above export PATH example, I've not specified any sbin/ directories, as they're not required for this script. So overall that should be pretty save, as a user won't be able to write to those directories

cytopia commented 3 years ago

@dmandala I've addressed it here: https://github.com/cytopia/linux-timemachine/pull/66

cytopia commented 3 years ago

@dmandala closing this in favour of #66 which has addressed this. In case you have other concerns, feel free to comment here.