I reviewed nlohmann-json3 3.11.3-2 as checked into plucky. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.
nlohmann-json3 is a library for managing JSON files that is designed to be
intuitive and with trivial integration. The library puts a strong emphasis
on these aspects along with testing as opposed to memory efficiency and
speed.
- CVE History
- None
- The library itself does not seem to have a CVE history. The library
has been affected by CVEs from other dependencies, to which upstream
maintainers seem to have responded to promptly. There have also been
CVE cases of other applications using this library as a dependency
due to incorrect handling of exceptions thrown by the library, and
not due to the library's fault.
- Due to this, there is no history as to how upstream handles security
issues directly. However, the system to tackle security issues should
they arise seems to be appropriate, with proper configuration for
reporting security issues and advisories.
- Build-Depends
- Normal build depends.
- pre/post inst/rm scripts
- None
- init scripts
- None
- systemd units
- None
- dbus services
- None
- setuid binaries
- None
- binaries in PATH
- None
- sudo fragments
- None
- polkit files
- None
- udev rules
- None
- unit tests / autopkgtests
- Now contains tests at build time. Also contains autopkgtests.
- Upstream project advertises 100% code coverage. This seems to be
mostly true (with 99% code coverage)
- cron jobs
- None
- Build logs
- Normal build logs.
- Processes spawned
- None
- Memory management
- Seems to have normal memory management. The code seems to use smart
pointers and allocates/deallocates memory normally.
- File IO
- None
- While the library may interact with arbitrary files and parse them,
the library seems to be adequately supported due to upstream putting
a strong emphasis on security and vulnerability scanning.
- Logging
- None
- Environment variable usage
- None
- Use of privileged functions
- None
- Use of cryptography / random number sources etc
- None
- Use of temp files
- None
- Use of networking
- None
- Use of WebKit
- None
- Use of PolicyKit
- None
- Any significant cppcheck results
- None
- Mostly warnings and issues with docs/tests.
- Any significant Coverity results
- Many coverity defects were identified. The coverity issues identified
do not seem like security vulnerabilities, but are rather design and
clean code centric.
- Use of auto that causes a copy
- These seem to be false positives as a copy in this instances is
required due to a removal of those variables later.
- Unchecked return
- There have been multiple defects of this type, where the return
of a function is not checked. These seem to be deliberate design
choices. The functions themselves make a memcpy and then return
whether the function succeeded or not. This seems like a minor
issue itself.
- Operands don't affect result
- These seem to be evaluated at compile time and therefore will
always be true regardless of the operand results. This again seems
like a minor issue related to clean code.
- Variable copied when it could be moved
- Many instances of these defects. These, again, seem like minor
issues.
- Resource leak in object
- Many instances of these defects as well. In this case, this could
result in a memory leak as constructors are called without
destructors. However, a function called destroy is part of the same
class, which seems to act like a destructor.
- Logically/structurally dead code
- Most instances identified by coverity were of this nature. As the
project is relatively large, it is expected to have instances of
unreachable code statements. In some cases, they are rather obvious,
and in others, it is unclear whether tackling these issues could
potentially introduce issues later down the road.
- Integer overflow defects
- Coverity has identified multiple integer overflow defects. From the
analysis, it seems like these are false positives, as either the
variable is always unsigned and therefore a conversion from a
negative value will not happen, or the `if` statements have clear
boundaries. Therefore, these were not deemed like issues.
- Memset fill value of '0'
- These seem intended as the code itself is working with characters,
and a memset of the character '0' rather than the number seems
intended.
- Parse Warning
- These seem to be calculated at runtime and are therefore fine to
trigger with a constant switch expression.
- Uncaught exception
- Multiple cases of these due to the deliberate usage of `noexcept`.
Some of these also have comments to remove warnings due to the
failure to trigger exceptions.
- Using a moved object
- Multiple cases of returning a value after std::move. They seem like
intended cases, and are therefore unlikely to cause issues if the
code uses the moved objects properly.
- These have also been multiple issues with tests which have been ignored
due to these not being intended to be ran in a production environment.
- Upstream claims to test their code with Coverity as well. Therefore, it
is likely that upstream is already aware of these issues.
- Any significant shellcheck results
- None
- Test related issues, unimportant.
- Any significant bandit results
- None
- Any significant Semgrep results
- None
The project seems to be relatively large in both code and popularity. While
there are many Github issues that have been open for a few years now, these are
addressed properly by maintainers. The repository advertises good test code
coverage, and constant vulnerability static and dynamic analysis through
tools such as sanitizers, coverity, and valgrind. The repository also claims
to implement OSS-Fuzz. While the library may interact with arbitrary files,
the project seems adequate when it comes to handling security. The repository
also contains a SECURITY.md file.
The code itself seems maintainable, with comments describing functions and
code lines wherever applicable. While some parts of the codebase may be more
difficult to understand than others (such as binary processing) the codebase
seems to be maintainable from a security standpoint.
Security team ACK for promoting nlohmann-json3 to main.
** Changed in: nlohmann-json3 (Ubuntu)
Status: New => In Progress
** Changed in: nlohmann-json3 (Ubuntu)
Assignee: Ubuntu Security Team (ubuntu-security) => (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
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
