#3292 Issue `open`: lib/sesam-functions.sh may source third party code¶

Labels: enhancement

jsmeix opened issue at 2024-07-23 09:22:¶

See
https://github.com/rear/rear/issues/3285#issuecomment-2244488622
which reads (excerpts):

usr/share/rear/lib/sesam-functions.sh

The crucial part is

sesam2000ini_file="/etc/sesam2000.ini"
...
source $sesam2000ini_file

so it does source a third party file as

source /etc/sesam2000.ini

I don't know the syntax of 'sesam2000.ini'
so I cannot mitigate it.

Perhaps /etc/sesam2000.ini can be trusted
to be sourced like other config files,
for example /etc/sysconfig/kernel and
/etc/dracut.conf and /etc/dracut.conf.d/*.conf cf.
https://github.com/rear/rear/issues/3285#issuecomment-2244555297

@rear/contributors
it should be verified before the ReaR 3.0 release
if third party code could be sourced here
or if it is reasonably safe.

abbbi commented at 2024-07-23 11:30:¶

hi,

the file /etc/sesam2000.ini basically just includes
the configuration variables that point to the installation
path of the installed sesam component and its configuration files.

It looks basically like this:

VERSION=5.1.0.51
SM_BIN_SESAM=/opt/sesam/bin/sesam/
SM_BIN_SMS=/opt/sesam/bin/sms/
SMS_INI=/var/opt/sesam/var/ini/sms.ini
STPD_INI=/var/opt/sesam/var/ini/stpd.ini
SM_INI=/var/opt/sesam/var/ini/sm.ini
SM_CTRL_INI=/var/opt/sesam/var/ini/sm.ini

It will not contain other things than these variable definitions.
Speaking as SEP employee, this file can be trusted.

jsmeix commented at 2024-07-23 11:59:¶

@abbbi
thank you for your prompt reply!

When the syntax is strictly only

VARIABLE=VALUE

it is not needed to 'source' it
so security could be improved (with reasonable effort)
to be safe even without the need to trust it
by not executing it but only parsing it
when for such simple syntax correct parsing
is sufficiently simple to be implemented like

myvar="$( grep '^VARIABLE=' /etc/sesam2000.ini | cut -d '=' -f2 )"

Or could it happen that a user specifies things like

  VERSION=5.1.0.51
SM_BIN_SESAM='/opt/sesam/bin/sesam/'
SM_BIN_SMS="$( echo /opt/sesam/*/sms/ )"
test -s /opt/sesam/var/ini/sms.ini && SMS_INI=/opt/sesam/var/ini/sms.ini || SMS_INI=/var/opt/sesam/var/ini/sms.ini

i.e. things like indentation or quotation
or command substitution or conditionals
which would basically require execution?

abbbi commented at 2024-07-23 12:34:¶

i.e. things like indentation or quotation or command substitution or conditionals which would basically require execution?

no

jsmeix commented at 2024-07-24 12:16:¶

@abbbi
would you agree when I make a pull request
to improve security by only parsing it
(instead of executing it)?

Or would you reject such a change and prefer
to execute it e.g. to be future proof against
things like indentation or quotation
or command substitution or conditionals
which would basically require execution?

jsmeix commented at 2024-07-25 07:42:¶

I removed the "critical/security/legal" label from this issue
because it is no longer "critical/security/legal", cf.
https://github.com/rear/rear/issues/3292#issuecomment-2244983803

abbbi commented at 2024-07-25 11:29:¶

@abbbi would you agree when I make a pull request to improve security by only parsing it (instead of executing it)?

Or would you reject such a change and prefer to execute it e.g. to be future proof against things like indentation or quotation or command substitution or conditionals which would basically require execution?

no objections from my side..

jsmeix commented at 2024-09-05 09:12:¶

I postponed it to the ReaR v3.1 milestone
because things are currently OK, see
https://github.com/rear/rear/issues/3292#issuecomment-2249670113
so this could be improved for ReaR v3.1 as time permits.

[Export of Github issue for rear/rear.]

#3292 Issue open: lib/sesam-functions.sh may source third party code¶