iniparser_getboolean() can segfault on malformed config files

[invd]

During recent fuzzing of the iniparser library via libFuzzer, I've discovered that iniparser can run into a segfault when parsing malformed configuration files.
As far as I can see, this is a local denial of service problem for some programs if attackers are able to provide or modify configuration files (and know which boolean configuration keys are fetched).

The following code position leads to the crash:

iniparser/src/iniparser.c

Line 557 in f858275

if (c[0]=='y' || c[0]=='Y' || c[0]=='1' || c[0]=='t' || c[0]=='T') {

Sanitizer crash info:

iniparser.c:561:9: runtime error: load of null pointer of type 'const char'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
iniparser.c:561:9 in
AddressSanitizer:DEADLYSIGNAL
=================================================================
==18912==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000055bd8c bp 0x7ffe80f4c070 sp 0x7ffe80f4bf80 T0)
==18912==The signal is caused by a READ memory access.
==18912==Hint: address points to the zero page.

The custom fuzzer harness is structured similarly to the iniexample.c example code. The relevant call after loading the target configuration tries to load the p:h key:

iniparser_getboolean(ini, "p:h", -1);

The following configuration file will lead to problems for the boolean fetch mentioned above:
[P:h]

Note: p:h was chosen arbitrarily as a shorter version of the pizza:ham key in the code example and has no special significance beyond that.

I've reported this finding privately to @ndevilla and @touilleMan.
@ndevilla has responded quickly and asked me to create a public issue report for it here.

[dofuuz]

There are similar problem in iniparser_getlongint() and iniparser_getdouble().

[lmoellendorf]

@invd

I tried to reproduce the issue without success. Maybe I am missing something.

Steps to reproduce:

1. build current iniparser from master
2. create file example.ini with this content:

[Pizza:ham]

3. execute:

./example/iniexample ./example.ini

This is the result:

./example/iniexample ./example.ini
[pizza:ham]=UNDEF
Pizza:
Ham:       [-1]
Mushrooms: [-1]
Capres:    [-1]
Cheese:    [-1]
Wine:
Grape:     [UNDEF]
Year:      [-1]
Country:   [UNDEF]
Alcohol:   [-1]

There is no segfault. What am I missing?

[dofuuz]

@lmoellendorf
It was already fixed with #146

[invd]

Good to see this is resolved now.

I'm trying to understand the reporting + patch history of this security issue.
It looks like

• I discover the problem with iniparser_getboolean() in 2020 and disclose it to the project (first internally, then publicly).
• @dofuuz reports in 2022 that iniparser_getlongint() and iniparser_getdouble() are also affected.
• @dofuuz proposes a patch PR Let iniparser_getstring() not return NULL if there is a section (not key) with specified name #139 in 2022, which is not merged.
• @NotmebutWind re-discovers (?) the issue pattern as NULL pointer cause crash in iniparser_getboolean. #144 in 2023 since it is not yet fixed.
• @xTeixeira proposes patch PR Handle null return from iniparser_getstring #146 which is accepted, resolving the issue in the master branch.
• CVE-2023-33461 is issued to cover the issue, as mentioned in NULL pointer cause crash in iniparser_getboolean. #144 (The CVE describes iniparser_getlongint(), but effectively covers all three variants).
• Issue tracking IDs for the Fedora package: bugzilla, FEDORA-2023-65f751f76d.
• As of today, there is no new official tagged patch release.

@ndevilla the v4.1 is very old, are you planning any new release?

[lmoellendorf]

@invd
Thank your for the summary.

Yes, @ndevilla is planning a new release and asked me to help me getting it ready.

The plan is to triage issues and bugs, do a first pass to correct the obvious, and create v4.2 ASAP.
All issues which are beyond bug fixing will have to wait for v5.

[ndevilla]

@lmoellendorf has kindly accepted to take the lead and process most issues and PRs on this project. Thanks a lot Lars!
The objective is to have a v4.2 release with known issues fixed, then move on to a v5 with possibly new features, breaking changes, a better build system, anything that would be needed. Hosting may move over to gitlab and this project frozen here as v4.2.