[Bug]: Declare the variable with eval fails if value contains space char about ini-file-parser HOT 10 CLOSED

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.

from ini-file-parser.

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

from ini-file-parser.

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)

from ini-file-parser.

New version released.

from ini-file-parser.

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.

from ini-file-parser.

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.

from ini-file-parser.

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.

from ini-file-parser.

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.

from ini-file-parser.

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.

from ini-file-parser.

I have actually gone in a slightly different direction:

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

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

escape the value BEFORE the eval, and then unescape it when displaying it or returning it to the caller.

This seems to work for all the test cases I have and even retains the '' in the value.

from ini-file-parser.