Review for Source Package: nlohmann-json3
[Summary]
I think the rationale needs to be more certain before being able to give a
finale MIR team ack. Check the Required TODOs for a list of them. Finally, as
the history of CVEs look like concerning, once those questions are settled, I
will request a security team review.
Notes:
Required TODOs:
- libboost-json is already in the archive, and is a popular library. I feel
there are obviously some duplication here and the reasoning why we can’t swap
it out or work on this dependency with upstream to use this one is not
articulated on the rationale.
- There is no team subscribed to the bug and no mention on who would maintain
it within Canonical. Please add that part to this request.
- does not have a test suite that runs at build time. A lot of tests are
included in the package and it seems some can run at build time and not only at
autopkgtests time. The description says that some need some stuff downloaded
from the Internet. Can some be embedded into the library?I may be wrong, but I
would like to see some analyzes around this.
Recommended TODOs:
- only one, small, repeated build error for the tests:
/<<PKGBUILDDIR>>/tests/src/make_test_data_available.hpp:21:60: warning:
ignoring attributes on template argument ‘int (*)(FILE*)’ [-Wignored-attributes]
| const std::unique_ptr<std::FILE, decltype(&std::fclose)>
file(std::fopen(TEST_DATA_DIRECTORY "/README.md", "r"), &std::fclose);
| ^
Would be nice to get it fixed.
[Rationale, Duplication and Ownership]
- libboost-json is already in the archive, and is a popular library. I feel
there are obviously some duplication here and the reasoning why we can’t swap
it out or work on this dependency with upstream to use this one is not
articulated on the rationale.
- There is no team subscribed to the bug and no mention on who would maintain
it within Canonical. Please add that part to this request.
[Dependencies]
OK:
- no other Dependencies to MIR due to this
- nlohmann-json3 checked with `check-mir`
- all dependencies can be found in `seeded-in-ubuntu` (already in main)
- none of the (potentially auto-generated) dependencies (Depends
and Recommends) that are present after build are not in main
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
more tests now.
[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard
[Security]
OK:
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not expose any external endpoint (port/socket/... or similar)
- 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)
- does not deal with security attestation (secure boot, tpm, signatures)
- does not deal with cryptography (en-/decryption, certificates,
signing, ...)
- this makes appropriate (for its exposure) use of established risk
mitigation features (dropping permissions, using temporary environments,
restricted users/groups, seccomp, systemd isolation features,
apparmor, ...)
Problems:
- history of CVEs is quite large. I would welcome some security review on it.
- does potentially parse data format from untrusted source, (json), which is
the core
of the library! Hence asking for the security review :)
[Common blockers]
OK:
- does not FTBFS currently
- does have a non-trivial test suite that runs as autopkgtest
- This does not need special HW for build or test
- no new python2 dependency
Problems:
- does not have a test suite that runs at build time. A lot of tests are
included in the package and it seems some can run at build time and not only at
autopkgtests time. The description says that some need some stuff downloaded
from the Internet. Can some be embedded into the library?I may be wrong, but I
would like to see some analyzes around this.
[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- Source only library for build time embedding, does not need symbol tracking.
- debian/watch is present and looks ok (if needed, e.g. non-native)
- Upstream update history is good
- Debian/Ubuntu update history is good
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
- no massive Lintian warnings
- debian/rules is rather clean
- It is not on the lto-disabled list
[Upstream red flags]
OK:
- no incautious use of malloc/sprintf (as far as we can check it)
- no use of user nobody
- no use of setuid / setgid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit or libseed
- not part of the UI for extra checks
- no translation present, but none needed for this case (user visible)?
Problems:
- only one, small, repeated build error for the tests:
/<<PKGBUILDDIR>>/tests/src/make_test_data_available.hpp:21:60: warning:
ignoring attributes on template argument ‘int (*)(FILE*)’ [-Wignored-attributes]
| const std::unique_ptr<std::FILE, decltype(&std::fclose)>
file(std::fopen(TEST_DATA_DIRECTORY "/README.md", "r"), &std::fclose);
| ^
Would be nice to get it fixed.
** Changed in: nlohmann-json3 (Ubuntu)
Status: New => Incomplete
** Changed in: nlohmann-json3 (Ubuntu)
Assignee: Didier Roche-Tolomelli (didrocks) => (unassigned)
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2093868
Title:
[MIR] nlohmann-json3
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/nlohmann-json3/+bug/2093868/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
