Comment 5 for bug 1913321

Revision history for this message
Alex Murray (alexmurray) wrote :

I reviewed iniparser 4.1-4 as checked into hirsute. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

iniparser is a small C library for parsing ini-style configuration files.

- CVE History:
  - None, however in 2016 a security issue was raised on their Github
    https://github.com/ndevilla/iniparser/issues/68 - it took a few months
    for this to be fixed but at worst this was a DoS issue - no CVE was
    ever assigned.
- No security relevant Build-Depends
- No pre/post inst/rm scripts
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- No binaries in PATH
- No sudo fragments
- No polkit files
- No udev rules
- unit tests / autopkgtests
  - autopkgtests run a couple of the examples shipped in the package source
  - unit tests are run during the build and these look reasonably
    extensive - there was one issue discovered by sbeattie via Coverity
    which prevented one of the tests cases in one of the tests from running
    https://github.com/ndevilla/iniparser/issues/131
- No cron jobs
- Build logs:
  - ERRORS / WARNINGS
    - During build gcc outputs the following warning:
      src/iniparser.c: In function ‘iniparser_load’:
      src/iniparser.c:791:32: warning: ‘__builtin___sprintf_chk’ may write a terminating nul past the end of the destination [-Wformat-overflow=]
    - This happens at the following code:

      sprintf(tmp, "%s:%s", section, key);

      In this case, where tmp, section and key are declared as:

      char section [ASCIILINESZ+1] ;
      char key [ASCIILINESZ+1] ;
      char tmp [(ASCIILINESZ * 2) + 1] ;

      As such, at most section and key are both ASCIILINESZ plus 1 colon
      separator fills then entire tmp buffer and leaves no space for a
      terminating NUL - so this looks like a real bug which could result in
      a 1-byte stack buffer overflow. This has already been fixed upstream
      in
      https://github.com/ndevilla/iniparser/commit/2412f165bcfde4ad8e3426fd59f2a920492b8c19
      so this patch should be integrated into our package.

  - LINTIAN FAILURES
    - Only 2 relevant lintian warnings:

      W: iniparser source: uses-deprecated-adttmp debian/tests/iniexample (line 4)
      W: iniparser source: uses-deprecated-adttmp debian/tests/parse (line 4)

- No processes spawned
- Memory management
  - Appears to be careful for the main library code (there are a bunch of
    issues in the unit test code but in general they are minor anyway)
- File IO
  - Opens whatever path is specified in main library function
    iniparser_load() - appears to have relatively strict validation of
    input from this
- No logging
- No environment variable usage
- No use of privileged functions
- No use of cryptography / random number sources etc
- No use of temp files other than in unit tests
- No use of networking
- No use of WebKit
- No use of PolicyKit

- No significant cppcheck results
- No significant Coverity results
- No significant shellcheck results
- No significant bandit results

In general iniparser appears reasonably safe and the upstream seems
relatively responsive to potential security issues. The code has reasonable
unit tests etc that should allow to easily re-test the library in the event
that it needs to be patched for any possible future security issue.

Security team ACK for promoting iniparser to main once the above listed
patch is integrated to fix this obvious issue.