Checking where ReaR sources files with '.' and trying to verify those cases

[jsmeix]

See https://github.com/rear/rear/issues/3260#issuecomment-2331033842 and https://github.com/rear/rear/issues/3260#issuecomment-2248252572 (excerpt)

usr/share/rear/skel/default/bin/dhclient-script:
                    . ${f}
            . ${ETCDIR}/dhclient-${interface}-down-hooks
            . ${ETCDIR}/dhclient-down-hooks
                    . ${f}

usr/share/rear/skel/default/etc/scripts/dhcp-setup-functions.sh:
        . ${ETCDIR}/dhclient-exit-hooks
            . ${ETCDIR}/dhclient-${interface}-up-hooks
            . ${ETCDIR}/dhclient-up-hooks

usr/share/rear/skel/default/etc/scripts/run-syslog:
    . /usr/share/rear/lib/layout-functions.sh

usr/share/rear/skel/default/etc/scripts/system-setup.d/00-functions.sh:
. /usr/share/rear/lib/global-functions.sh
. /usr/share/rear/lib/network-functions.sh

usr/share/rear/restore/GALAXY/default/400_restore_with_galaxy.sh:
       . ./GxCmd

[jsmeix]

Regarding

usr/share/rear/skel/default/bin/dhclient-script:
                    . ${f}
            . ${ETCDIR}/dhclient-${interface}-down-hooks
            . ${ETCDIR}/dhclient-down-hooks
                    . ${f}

usr/share/rear/skel/default/etc/scripts/dhcp-setup-functions.sh:
        . ${ETCDIR}/dhclient-exit-hooks
            . ${ETCDIR}/dhclient-${interface}-up-hooks
            . ${ETCDIR}/dhclient-up-hooks

According to

# git log -p --follow usr/share/rear/skel/default/bin/dhclient-script | egrep '^commit| \. '
...
commit 38d5bd280654dd4e05a8a408daad8e08925c3ab0
+                    . ${f}
+            . ${ETCDIR}/dhclient-${interface}-down-hooks
+            . ${ETCDIR}/dhclient-down-hooks
+                    . ${f}

# git log -p --follow usr/share/rear/skel/default/etc/scripts/dhcp-setup-functions.sh | egrep '^commit| \. '
...
commit 38d5bd280654dd4e05a8a408daad8e08925c3ab0
+        . ${ETCDIR}/dhclient-exit-hooks
+            . ${ETCDIR}/dhclient-${interface}-up-hooks
+            . ${ETCDIR}/dhclient-up-hooks

sourcing via '.' in usr/share/rear/skel/default/bin/dhclient-script and usr/share/rear/skel/default/etc/scripts/dhcp-setup-functions.sh originate from https://github.com/rear/rear/commit/38d5bd280654dd4e05a8a408daad8e08925c3ab0 which has only this commit message

Added DHCP client support to rear. Sponsored by J&J.

but this commit is also where usr/share/rear/lib/network-functions.sh originated and this contains

+# set of functions that will be used by our own implementation
+# of dhclient-script, but these can/could be used by other
+# scripts as well
+#
+# Most of the functions are coming from the fedora dhclient-script

So I guess that also usr/share/rear/skel/default/bin/dhclient-script and usr/share/rear/skel/default/etc/scripts/dhcp-setup-functions.sh came from Fedora at the time of that commit i.e. on Dec 10, 2010

On the one hand this indicates that this code in ReaR could be OK (because we may assume Fedora code is OK).

On the other hand this code in ReaR is rather old (more than 13 years meanwhile) and I assume possible (security) bug fixes at Fedora were not backported into ReaR.

This code in ReaR should be checked if it is considered to be still sufficiently OK for what ReaR needs (DHCP client support in the ReaR recovery system) so I filed https://github.com/rear/rear/issues/3320

[pcahyna]

I would like to use some more sophisticated parser to identify this use of ., but I have not started this yet.

If you want to make sure that no . is used and source is used instead, enable -n . is your friend. The bash manual says about . that "This builtin is equivalent to ‘source’.", but they can be enabled or disabled separately:

$ cat print_foo 
echo foo
$ enable -n .
$ source print_foo 
foo
$ . print_foo 
bash: .: command not found

[jsmeix]

Regarding

usr/share/rear/skel/default/etc/scripts/run-syslog:
    . /usr/share/rear/lib/layout-functions.sh

usr/share/rear/skel/default/etc/scripts/system-setup.d/00-functions.sh:
. /usr/share/rear/lib/global-functions.sh
. /usr/share/rear/lib/network-functions.sh

/usr/share/rear/lib/layout-functions.sh /usr/share/rear/lib/global-functions.sh /usr/share/rear/lib/network-functions.sh are ReaR's own files so no third-party file is sourced here.

I will replace '.' by 'source' - done right now via https://github.com/rear/rear/commit/d19d51993853e763c1aba5f9828075acf0dfdf07 and https://github.com/rear/rear/commit/a1bcb21a54cb27a7e2ce5564a7ef05b79c5cdacd

[jsmeix]

Regarding

usr/share/rear/restore/GALAXY/default/400_restore_with_galaxy.sh:
       . ./GxCmd

From plain looking at the code I cannot make sense of it. In particular 'GxCmd' appears only in usr/share/rear/restore/GALAXY/default/400_restore_with_galaxy.sh (excerpts)

# GxCmd checks for $# and calls the GxCmdLine script if there are any cmdline args.
...
        . ./GxCmd
...
        ./GxCmd -cmd restore ...

so it seems GxCmd (and GxCmdLine) belong to the BACKUP=GALAXY software. In this case it is probably OK to source GxCmd because the BACKUP=GALAXY software can be trusted. Nevertheless it should be verified whether or not my above assumption that '. ./GxCmd' can be trusted actually holds so I filed https://github.com/rear/rear/issues/3321

I will replace '.' by 'source' - done right now via https://github.com/rear/rear/commit/f3de9e84949f181b5eb8ab8a1e702e87c5d236d5

[jsmeix]

All cases that I found and listed in this issue are now done so I close this issue as done.

For possibly further cases where '.' is used to source files, see https://github.com/rear/rear/issues/3260#issuecomment-2355192852 and https://github.com/rear/rear/issues/3260#issuecomment-2355205283

[jsmeix]

@pcahyna regarding your https://github.com/rear/rear/issues/3319#issuecomment-2355033810 in particular the part about enable -n .

'man bash' reads

enable [-a] [-dnps] [-f filename] [name ...]
  Enable and disable builtin shell commands.
  Disabling a builtin allows a disk command
  which has the same name as a shell builtin
  to be  executed ...

Could it be ever possible that a disk command named '.' can exist? I think it cannot because '.' is the current directory but I don't know for sure.

In ReaR we use already in usr/share/rear/lib/_input-output-functions.sh

# Make sure nobody else can use trap:
function trap () {
    BugError "Forbidden usage of trap with '$*'. Use AddExitTask instead."
}

which seems to also work:

# function . () { echo "Forbidden usage of '.'" ; }

# type -a .
. is a function
. () 
{ 
    echo "Forbidden usage of '.'"
}
. is a shell builtin

# . print_foo
Forbidden usage of '.'

# builtin . print_foo
foo

Perhaps it is better to use a '.' function? In both cases one can still call the shell builtin '.' via 'builtin':

# unset -f .

# type -a .
. is a shell builtin

# enable -n .

# type -a .
bash: type: .: not found

# builtin . print_foo
foo

[jsmeix]

I think it is better to use a '.' function instead of enable -n . because with a function we can implement what should happen when '.' is used (e.g. BugError or whatever else we like).

I tested with current GitHub master code if '.' is used in my personal test case by adding at the beginning of sbin/rear

function . () { echo "Forbidden usage of '.'" ; exit 99 ; }

My personal test case etc/rear/local.conf

OUTPUT=ISO
BACKUP=NETFS
BACKUP_OPTIONS="nfsvers=3,nolock"
BACKUP_URL=nfs://192.168.178.66/nfs
REQUIRED_PROGS=( "${REQUIRED_PROGS[@]}" snapper chattr lsattr )
COPY_AS_IS=( "${COPY_AS_IS[@]}" /usr/lib/snapper/installation-helper /etc/snapper/config-templates/default )
BACKUP_PROG_INCLUDE=( /boot/grub2/x86_64-efi /boot/grub2/i386-pc /home /opt /root /srv /tmp /var /usr/local )
POST_RECOVERY_SCRIPT=( 'if snapper --no-dbus -r $TARGET_FS_ROOT get-config | grep -q "^QGROUP.*[0-9]/[0-9]" ; then snapper --no-dbus -r $TARGET_FS_ROOT set-config QGROUP= ; snapper --no-dbus -r $TARGET_FS_ROOT setup-quota && echo snapper setup-quota done || echo snapper setup-quota failed ; else echo snapper setup-quota not used ; fi' )
SSH_ROOT_PASSWORD="rear"
USE_DHCLIENT="yes"
FIRMWARE_FILES=( 'no' )
MODULES=( 'loaded_modules' )
PROGRESS_MODE="plain"
PROGRESS_WAIT_SECONDS="5"
USE_SERIAL_CONSOLE=''

on a SLES15-SP6 test VM with 15 GiB disk and default btrfs structure.

I tested the following workflows

• mkbackup
• recover (on another test VM)
• help
• checklayout
• dump
• savelayout
• shell
• validate

All worked for me in my personal test case. Of course this cannot check if '.' is used for other use cases.

[jsmeix]

While thinking about using a '.' function to forbid usage of '.' it crossed my mind that we could also use a 'source' wrapper function so normal usage of 'source' in scripts calls our 'source' wrapper function so we get a central place in ReaR where we keep control over what is sourced and where we could implement what we need like path-based checks or whatever else (e.g. log details in debug mode).

Of course this is not for ReaR 3.0 but for a ReaR version after ReaR 3.0.

What I will never ever do again is to manually search in a clumsy and annoying way through all our code where something is sourced.

I did a quick proof of concept with current GitHub master code by adding at the beginning of sbin/rear

function source () { echo "Sourcing '$*'" 1>&2 ; builtin source "$@" ; }

on the same SLES15-SP6 test VM with 15 GiB disk and default btrfsstructure as above and "rear -D mkrescue" worked for my particular test case.

Redirection to stderr is mandatory in the 'source' function otherwise things badly error out in 400_copy_modules.sh which happens - as far as I see at first glance - because of usr/share/rear/rescue/GNU/Linux/220_load_modules_from_initrd.sh

# Fedora, Red Hat & new SUSE uses dracut
if test -s /etc/dracut.conf ; then
    MODULES_LOAD+=(
        $(
            add_drivers=
            source /etc/dracut.conf
            for s in /etc/dracut.conf.d/*.conf ; do
                source $s
            done
            echo $add_drivers
        )
    )
fi

cf. https://github.com/rear/rear/issues/3285#issuecomment-2244555297

So stdout of 'source ...' gets added to MODULES_LOAD which makes 400_copy_modules.sh error out with

Copying only currently loaded kernel modules (MODULES contains 'loaded_modules') and those in MODULES_LOAD
ERROR: Sourcing loaded or to be loaded but no module file?

when 'Sourcing' appears on stdout of the 'source' wrapper function.

By the way: I found the reason why 400_copy_modules.sh errors out rather fast and easily via

# grep '^Sourcing ' var/log/rear/rear-localhost.log | grep -v 'usr/share/rear/'
Sourcing '/root/rear.github.master/etc/rear/local.conf'
Sourcing '/etc/dracut.conf'
Sourcing '/etc/dracut.conf.d/10-persistent_policy.conf'
Sourcing '/etc/dracut.conf.d/99-debug.conf'
Sourcing '/etc/dracut.conf.d/ostree.conf'

i.e. the /etc/dracut.conf* files are the only ones we 'source' which look "suspicious" ;-)