go-ini
Escaping is broken
Please give general description of the problem
Go-ini does not properly cope with characters that need to be escaped.
Please provide code snippets to reproduce the problem described above
- Let's look at key name escaping:
https://github.com/go-ini/ini/blob/3be5ad479f69d4e08d7fe25edf79bf3346bd658e/file.go#L332-L339
Say you have a key name which contains a " and a `, lines 335 and 336 will mean that you wrap the key in `, however, leading to an unescaped component. So for example the key:
A pathological `c"as"`e
becomes:
`A pathological `c"as"`e`
Which is likely to be interpreted as:
A pathological case
Let's try a value injection say you provide a key named like this:
KEY`=FAKE_VALUE ;"`
This becomes:
`KEY`=FAKE_VALUE ;"`` = ACTUAL_VALUE
If say you simply promote the """ case higher - then you're not protecting against embedded """ strings within the key.
I presume that key names are not allowed to have newlines in them and that this is prevented elsewhere. If not then this is also a major problem.
- Key values
These are more of a problem because these can contain new lines and are far more likely to be exploited as most users of this library will not allow arbitrary keys.
https://github.com/go-ini/ini/blob/3be5ad479f69d4e08d7fe25edf79bf3346bd658e/file.go#L358-L366
A value containing """\n can rewrite arbitrary sections of the config file.
e.g. the value
VALUE"""
[ANOTHER SECTION]
KEYS=VALUES
...
; more arbitrary content
; """
Do you have any suggestion to fix the problem?
If there is no proper mechanism for escaping these cases, (I suspect that in general that this is the case as ini files are very poorly documented - but I guess an option could given to backslash escape) go-ini should simply return an error saying that there is no safe delimiter.
Check that the chosen delimiter is not included in the value and not just assume that because one delimiter is included you can just use another without checking.
I believe a test that covers the cases above should be added, which attempts to load the file using parse_ini_file via PHP.
Something like:
<?PHP
$ini_array = parse_ini_file("test.ini");
print_r($ini_array);
Where test.ini is generated via SaveTo and contains keys and values and sections with cases for all the above escape flaws.
I'm unsure if this fits every general case, but in some preliminary testing I have found that replacing the entire value escape section with golang stdlib val = strconv.Quote(val) escapes the values much better, with the only downside of not allowing newlines (newlines in the value string becomes the string \n) which depending on your use case may be desired, but could cause problems for uses where newlines are desired.
I imagine relying on a stdlib library, we are going to avoid a lot of the hard work and edgecase flaws we are seeing above.
Or alternatively allow a no-escape option where it will use the string from val directly so we can do our own escaping before adding the value.
The issue I hit is we were using strconv.Quote before passing values to this library, but the value had a backtick (`) which meant """ was added to either side, leading us to a value with 4 quotes each side and an unparsable ini file
If using strconv.Quote directly is not possible (as it escapes some characters that may not be desired), the source code would be a good guide to escaping characters https://github.com/golang/go/blob/master/src/strconv/quote.go