non-blocking items from MIR of iniparser: tests, 1 byte stack overflow

[Dan Streetman]

The following changes were requested as part of the MIR of libiniparser:

1) build does not run tests in test/ directory Edit

The top level makefile contains a target 'check' which runs the tests under the test/ directory, but the 'check' target is not run during the build (and make is not run for the test/ directory manually either).

Probably dh_auto_test should be overridden to also run 'make check' in the debian/rules file.

2) cherry-pick patch for 1-byte stack buffer overflow

- 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.

[Dan Bungert (dbungert)]

On analysis of this issue, I found that the upstream tests are being run - it's just that failures go unnoticed due to the test runner unconditionally returning exit code 0, pass or fail. By enhancing this runner to return exit code 1 upon test failure, we see the test failure log without having to do anything special.

Also improve the cleanup target while I'm here.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "Error out on failing test, and improve cleanup target" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Dan Bungert (dbungert)]

Security team has provided feedback, indeed there is a patch to merge. Let's not merge this one yet, and instead get this + the security patch in one go.

[Dan Bungert (dbungert)]

This patch addresses both items.

[Launchpad Janitor (janitor)]

This bug was fixed in the package iniparser - 4.1-4ubuntu2

---------------
iniparser (4.1-4ubuntu2) hirsute; urgency=medium

  * Genericize long int tests to accomodate where sizeof(long int) != 8
    (LP: #1918456)

iniparser (4.1-4ubuntu1) hirsute; urgency=medium

  * Change tests so that a test failure will stop the build (LP: #1915866)
  * Improvements to build steps such that objects from test get cleaned up
  * Incorporate upstream commit 2412f165bcfde4ad8e3426fd59f2a920492b8c19 from
    PR #104 to address likely 1 byte stack buffer overflow
    (thanks Edik Ponomarenko)

 -- Dan Bungert <email address hidden> Tue, 16 Mar 2021 11:42:53 -0600