#3437 PR `merged`: Allow sourcing via dot in non-ReaR files¶

Labels: bug, fixed / solved / done

jsmeix opened issue at 2025-03-25 09:30:¶

Type: Bug Fix
Impact: High

Fix the current regression since
https://github.com/rear/rear/pull/3434
that using the POSIX compliant '.' is forbidden.

Reference to related issue (URL):

https://github.com/rear/rear/pull/3434#issuecomment-2742598092
reads (excerpts):

Because '.' is the POSIX standard command
POSIX compliant third party software rightfully uses '.'
so ReaR must not enforce an environment where '.' is
forbidden in general.
...
The right solution is a '.' wrapper function which enforces
that '.' is not used in a ReaR script or in a ReaR config file
but which allows '.' in any non-ReaR files.

What ReaR still can enforce is an environment
where all what is executed via 'source' or '.'
is checked to be trustworthy, i.e. for each
file that is executed via 'source' or '.'
its owner must be one of the TRUSTED_OWNERS and
it must be located below one of the TRUSTED_PATHS.

How was this pull request tested?

"rear mkrescue" plus "rear mkbackuponly" on a SLES15-SP6 VM
and then on another VM "rear recover" worked for me
with OUTPUT=ISO and BACKUP=NETFS

Description of the changes in this pull request:

Added a '.' wrapper function which enforces
that '.' is not used in a ReaR script or in a ReaR config file
(via a new global array REAR_SOURCE_PATHS)
but which allows '.' in any non-ReaR files.

That '.' wrapper function calls the 'source' wrapper
to enforce that for all what is executed via 'source' or '.'
its owner must be one of the TRUSTED_OWNERS and
it must be located below one of the TRUSTED_PATHS.

The new global array REAR_SOURCE_PATHS
is not documented in default.conf
because it is at least currently
not meant as a user config variable.

Nevertheless to provide final power to the user
the use can set REAR_SOURCE_PATHS as he likes,
in particular with empty REAR_SOURCE_PATHS=()
the '.' wrapper function does no longer enforce
that '.' is not used in a ReaR script or in a ReaR config file.

pcahyna commented at 2025-03-25 13:29:¶

I have thought of a different approach:
introduce another helper (let's call it SourceExternal) that would be used by ReaR code whenever it sources external code (outside ReaR). This helper would remove the . wrapper (and restore it back after sourcing completes). This would of course require us to find and replace all uses of source for external code, but it would have the advantage of being more explicit what the sourcing is for (eventually we would have mostly SourceExternal and Source in our code and almost no plain source left).

jsmeix commented at 2025-03-25 14:28:¶

@pcahyna
sigh - I already explained it several times:

It cannot work to use whatever differently named function
to generically gain control over usage of 'source' and '.'
in whatever obscure code places which are in practice
outside of our control (e.g. lenghty contributions
to support a new backup method and things like that
where a proper review is impossible in practice - too easily
a single dot within some weird code could be overlooked)
exactly because we cannot reliably find all code places
where external code could be sourced via 'source' or '.'
(or 'eval' - but this is another separated topic).

I already had to try myself to
"find all code places where external code is sourced"
both for 'source' and '.' where I experienced myself
it is an annoying discouraging and disappointing task
(in particular because one never knows when it is done
i.e. that one has really found all cases where external
code is sourced) and this task would have to be done
again and again ad nauseam to continuously ensure that
ReaR does not by accident source unwanted stuff,
cf. https://github.com/rear/rear/issues/3259

By the way:
We have still those open / unfinished issues
https://github.com/rear/rear/issues/3321
https://github.com/rear/rear/issues/3295
https://github.com/rear/rear/issues/3292
https://github.com/rear/rear/issues/3291
from my manual attempts where I tried to find
all code places where external code might be sourced.

pcahyna commented at 2025-03-25 18:26:¶

@jsmeix thanks for the explanation. I see now that

obscure code places which are in practice
outside of our control (e.g. lenghty contributions
to support a new backup method and things like that
where a proper review is impossible in practice - too easily
a single dot within some weird code could be overlooked)

(emphasis mine) is the key problem, i.e. code that is part of our code base so in principle it is under our control, but in practice we can not commit ourselves to audit it all.

Another idea, similar but without needing a review of all places where source is used - would it make sense to detect inside source whether the sourced script is inside ReaR sources and if not, disable the . wrapper for the duration of the sourced file (and then reenable it)? The detection would be done in a similar way to what your currently have in the . wrapper. This would effectively move the complication from the . wrapper to the source wrapper.

This is merely an idea for discussion, I don't know myself which option is better. It just seems to me that running external code with one wrapper less might be preferable.

jsmeix commented at 2025-03-26 14:19:¶

To generically gain control over usage of 'source' and '.'
we need wrappers for both anyway and all the time.

I think if we implement to disable the '.' wrapper
under whatever conditions, those conditions could
have security holes which might be exploited
so a plain dot '.' could be the bash builtin
without TRUSTED_OWNERS / TRUSTED_PATHS protection
(I assume we can detect when 'builtin .' is used).
In contrast if we keep the '.' wrapper all the time
we do not need to worry about how to 100% correctly
disable and re-enable it.

My current implementation is my first attempt
to implement the desired functionality so that
it works sufficiently well for now.
I cannot predict the future so I cannot know
what changes will be needed in practice over time.
Things will of course get adapted and enhanced
in the future as needed.
Because all that are internal things in ReaR
(except names and meaning of user config variables)
we can change internal things in ReaR as we like.

jsmeix commented at 2025-03-26 16:27:¶

As there are no hard objections
I "just merged it" right now because
this week and perhaps also next week
(currently I cannot tell how long it takes)
I cannot work on ReaR because I have to fix
several security bugs in other software...

[Export of Github issue for rear/rear.]

#3437 PR merged: Allow sourcing via dot in non-ReaR files¶