4.0.x - crash when reading config file - configuration item with no value

[nchaigne]

Issue type

• Defect - Crash or memory corruption.

Defect

When reading configuration from file, a configuration error can crash the server:
If it's a known configuration item, but no value is provided (it should be an error but not crash).

Crash occurs in function cf_pair_parse_value (line 161 file cf_parse.c) here (because cp->value is NULL):

	if ((cp->value[0] == '\0') && cant_be_empty) {

[nchaigne]

Mmh that's not sufficient... now I have another crash, still in cf_pair_parse_value:

*(uint64_t *)out = strtoull(cp->value, NULL, 0);

[alandekok]

On Sep 25, 2019, at 10:21 AM, Nicolas C ***@***.***> wrote: Mmh that's not sufficient... now I have another crash, still in cf_pair_parse_value: *(uint64_t *)out = strtoull(cp->value, NULL, 0);

What CONF_PARSER is this for? The defaults should work. I.e. if a config item is required, then the parser should complain if it’s missing. If a config item isn’t required, then the source code should set a default value. It looks like you’re not setting a default value in either the source code or the config. So yes it will crash. I’ll push a fix so that it will return an error if there’s no cp->value when its passing something. This is really a coding error.

[nchaigne]

That's not a coding error, but a configuration error (not the default which is fine).
I agree it won't happen if the configuration is correct. But if is not, it's better if the server don't crash. :)

[nchaigne]

There are other issues maybe related to this. For example if you configure module linelog with (an obvious configuration error):

file {
    #filename = ${logdir}/linelog
    filename
}

Then the server crashes (because xlat_tokenize_literal is called with NULL "fmt").
I have the backtrace for this case, if it can help ?

[alandekok]

Hmm... OK. I'll put in patches then for this. The short answer (as always) is "don't do that" :)

[nchaigne]

Yes of course. I just accidentally botched the configuration and noticed the crash, I was not trying to make it happen :)

[nchaigne]

Rebuilt with latest HEAD, confirm issue is fixed.
Thanks!