[Bug 1913321] Re: [MIR] iniparser (dependency of mtd-utils)

[Alex Murray]

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.


** Bug watch added: github.com/ndevilla/iniparser/issues #68
   https://github.com/ndevilla/iniparser/issues/68

** Bug watch added: github.com/ndevilla/iniparser/issues #131
   https://github.com/ndevilla/iniparser/issues/131

** Changed in: iniparser (Ubuntu Hirsute)
     Assignee: Ubuntu Security Team (ubuntu-security) => (unassigned)

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to mtd-utils in Ubuntu.
Matching subscriptions: foundations-bugs-with-comments, mtd-utils
https://bugs.launchpad.net/bugs/1913321

Title:
  [MIR] iniparser (dependency of mtd-utils)

Status in iniparser package in Ubuntu:
  New
Status in mtd-utils package in Ubuntu:
  Invalid
Status in iniparser source package in Hirsute:
  New
Status in mtd-utils source package in Hirsute:
  Invalid

Bug description:
  [MIR] iniparser (dependency of mtd-utils)

  
  [Availability]
  ✓ The package is in universe.

  [Rationale]
  ✓ The package is a new build dependency of a package that we already
    support (mtd-utils).

  [Security]
  ✓ No CVEs
  ✓ No openwall
  ✓ No security relevant binaries
  - The github has several items of interest - commits not yet in Debian /
    Ubuntu that address buffer overflows, not-yet-merged fixes for missing
    null pointer checks/memory leaks, plus more issues filed with typical C
    code null checks / off by ones.  Could be OK with some updates to
    address the known issues.

  [Quality assurance]
  ✓ Used package with minimal effort.  Provides a doc package, and the
    header file for the lib has the same content.  API behaves mostly as
    expected and was easy to use just based on the header file.
  ✓ No debconf usage
  ✓ No long-term usability affecting bugs
  ✓ No Debian/Ubuntu bugs aside from this MIR
  - Upstream bugs of interest present, see security section above
  - Packaging in Debian seems mostly fine, but I noted that back-to-back
    invocation of dpkg-buildpackage fails.  A `make -C test clean` would
    resolve this.
  ✓ No exotic hardware expectations
  - While a test suite is present, failures in it are not failing the build.
  ✓ debian/watch file present
  - lintian --pedantic reports 6 items total, the most severe of which are 2
    warnings
  ✓ No reliance on obsolete/pending-demote packages

  [Dependencies]
  ✓ Dependencies are very modest and already in main. (libc6, and
    libjs-jquery for doc package)

  [Standards compliance]
  ✓ FHS looks good to me.
  ✓ Outstanding patches - there is a CMake patch, but upstream doesn't want 
    it.
    https://github.com/ndevilla/iniparser/blob/master/FAQ-en.md#your-build-system-isnt-portable-let-me-help-you
  - Recommended item DEB_BUILD_OPTIONS isn't explicitly implemented, all 6
    options currently listed are potentially relevant.
    https://www.debian.org/doc/debian-policy/ch-source.html#s-debianrules-options
  - The standards version is old https://tracker.debian.org/pkg/iniparser ,
    however v4.3.0 is an appropriate version for the last time the package
    was uploaded.

  [Maintenance]
  ✓ foundations-bugs subscribed on 
    https://bugs.launchpad.net/ubuntu/+source/iniparser/+subscriptions
  ✓ I consider this a "simple" package which should continue to be in sync
    with Debian

  [Background information]
  ✓ Package description is appropriate
  ✓ No recent (or ever) renames

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/iniparser/+bug/1913321/+subscriptions