[Bug]: Declare the variable with eval fails if value contains space char

[vollmeier-dev]

What happened?

Declare the variable with eval (line 256) fails if the value contains a space char (ascii 0x20).

For example, the following ini file:
[section1]
value1=abc def

Results to the following error:
ini-file-parser.sh: line 256: def: command not found

Expected behavior

Values containing spaces should work.

How do we reproduct the bug?

Parse a ini file with a value that contains a space char

Relevant log output

No response

Screeenshots

No response

Additional information

In line 256, enclose the value varaible in double quotes and it works:

eval "${section}_${key}=\"${value}\""

Code of Conduct

• I agree to follow this project's Code of Conduct

[vollmeier-dev]

By the way it's also a security issue. Instead of abc def you can have abc rm /etc/* (or another command) as the value. Then it executes the command and this is very bad, if you execute the script with elevated privileges. I think this needs further investigation, maybe complex commands (eg, sending an email with information) are possible with eval in this situation. Using eval with unfiltered input can be dangerous.

[lupraxus]

Thanks for the bug report, I will look into this now and let you know what I find.

[lupraxus]

If you want to have a space in value you achieve that by enclosing the value in single quotes, E.g.

name='This value has spaces in it'

The change you suggested still requires you to enclose the string in single quotes in order for spaces to be honoured but it does remove the error message and also helps mitigate any possible security issues (I couldn't find a way to exploit this)

[lupraxus]

New version released.

[vollmeier-dev]

When you change the line 255 the same was as line 256, then you don't use the single quotes for values with spaces.
But also when you change the line 255 too, i can still inject a command into your script with a value in the ini file.
You can use $(rm /etc/*) as the value. Eval will execute this command. You can test this by using $(ls / > ~/out.txt) as the value. You will see, that running you parser script it creates the out.txt file. So you can execute other (maybe dangerous) commands in this way.
As i say, eval and output from outside, can be very dangerous combination. It's still a security related issue, that can be exploited and it's also very easy to exploit this.
That definitely needs further investigation.

[lupraxus]

Thanks for the update, it was late and my brain wasn't engaged after Christmas/new year :)

The exploit as you say it is pretty simple. I have solved this by changing both lines 255 and 256, and the solution was to encapsulate both ${value}s with single quotes to stop them from being expanded and instead treated as literal strings.

I added a test exploit to the demo config (based on your ls example) - and it now displays this instead of executing it.

[vollmeier-dev]

I can still inject a command over a value in the ini file in your script (at line 254 and 255). I can close the single quote simply by start with a single quote (that creates a first empty string surrounded by single quotes) followed by the command and then again a single quote at the end (to create a second empty string surrounded by single quotes). Then the command executes twice at line 254 and 255.

The following value executes the command twice (as you can see in the file out.txt, there are to listing), but there is an error on line 255 (line 255: : command not found):
' $(ls / >> ~/out.txt) '

The following command will also execute the command twice, but without error:
' $(ls / >> ~/out.txt && echo test) '

For example, you can also add a check if the file already exist and only execute the command that write the file once (to demonstrate a little more complex command:
' $(test -e ~/out.txt || echo $(date +%Y%m%d_%H%M%S_%N) >> ~/out.txt && echo test) '

This are some possibilities, but you only need the first one (' $(ls / >> ~/out.txt) ') to exploit this issue and with the second one (' $(ls / >> ~/out.txt && echo test) ') you can hide the error message.

As you can see, it's still a security issue. To say it again, the eval command with input from outside is dangerous and tricky. I hope you understand the trick. It's something like as on sql injection, Neutralizing the single quotes with adding single quotes at the start and at the end of the value.

I think there is nothing to avoid filtering the input in a suitable way or not using the eval command (if this is possible). I would prefer the second option, but of course it's your decision.

[lupraxus]

The biggest issue is that the use of eval is needed in order to generate the dynamic variables in a clean portable way. There are other ways to do it but not in such a simple and easily portable way.

I will have a look at what other filtering / escaping I can apply to the right-hand side to mitigate the issues further.

[lupraxus]

Ok, I have done some more testing and this additional code seems to mitigate the issue:

function escape_string()
{
echo "$1" | sed -e 's/'''/'\"'/g'
}

I tested it with multiple values:

value4='$(ls / >> ~/out.txt)'
value5=$(ls / >> ~/bob.txt)
value6="$(ls / >> ~/bob2.txt)"

All of which failed to execute and were correctly displayed and processed. This should stop people from being able to step outside of the '' on lines 254 and 255.

I will do some additional testing, with quotes strings within the value etc.