Comment 3 for bug 1913321

Revision history for this message
Dan Streetman (ddstreet) wrote :

[Summary]
While there is an existing package in main that provides a C INI
parsing library, the mtd-utils package (already in main since at
least bionic) has simply embedded this library (and it continues
to do so). The only change was a patch added to mtd-utils late
last year to link to the system libiniparser.so instead of using
the embedded iniparser code. So the duplication seems necessary,
as changing mtdutils over to use a different ini parser library
would be a significant unnecessary amount of work (and unlikely
for upstream mtd-utils to accept).

There are some concerns listed under notes, but none required.
So this is ACK from MIR team.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main:
- libiniparser1

Notes:
I noted 2 additional concerns, but considering the fact that this
library code has been embedded in the mtd-utils package, in main,
since bionic, I don't believe either should block MIR.

Recommended TODOs:
- The build should run 'make check', or otherwise run the tests
  in the test/ directory; a failure in those tests (or in the
  CMake test) should fail the build
  Note: opened LP:#1915866 for this
- The apparent slow/stopped pace of updates upstream is concerning;
  Debian and/or Ubuntu should pull any upstream commits not currently
  included, as well as reviewing upstream bugs for possible patching
  in Debian/Ubuntu

[Duplication]
- existing main package 'libini-config5'
  provided by source package 'ding-libs'

[Dependencies]
OK:
- no other Dependencies to MIR due to this

Problems:
- -dev and -doc packages that need exclusion

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking (but does provide static libs)

[Security]
OK:
- history of CVEs does not look concerning (no CVEs)
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)

Problems:
- does not parse data formats, but does parse INI (config) format
- upstream has some open bugs possibly security-related

[Common blockers]
OK:
- does not FTBFS currently
- The package has a team bug subscriber (foundations)
- no translation present, but none needed for this case (not user visible)
- not a python/go package, no extra constraints to consider in that regard

Problems:
- has 2 test suites, but only runs 1 at build time
  - runs CMake test, but does not run test suite in test/ dir

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place
- d/watch is present and looks ok
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- Does not have Built-Using
- not go package

Problems:
- Upstream update history is slow/stopped
- Debian/Ubuntu update history is slow, but hard to determine because last upstream release was 2017

[Upstream red flags]
OK:
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks

Problems:
- Errors/warnings during the build
- questionable, or at least too casual, use of sprintf; warnings printed during build
- improper use of malloc inside built-in 'xstrdup' function, due to calling functions
  failing to check NULL return value
- some important open bugs (potential segfault, etc) Upstream