Comment 14 for bug 1024435

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

MIR review:
 * Builds fine with just main
 * extensive testsuite enabled in the build
 * Ubuntu does not carry a delta
 * Does not ship a symbols file or use an empty argument to dh_makeshlibs -V. This should be considered, but is not a blocker.
 * Does not have a bug subscriber
 * Does have a watch file
 * Releases are regular
 * Lintian clean
 * debian/rules is reasonable
 * build log is free of compiler warnings/errors
 * There are no open bugs in Debian or Ubuntu (other than this MIR :)

Security review:
No CVE history. I agree with Michael that it is undesirable to have multiple PDF libraries in main, but I am encouraged by Tobias and Jay's comments, in particular the bounds-checking and previous static analysis that Jay mentioned and what Tobias said about transformations not actually needing to examine/parse most of the PDF. The fact that Jay is the Debian maintainer and upstream and the software is mature and has been in Debian for several years and there are no upstream red flags.

Furthermore, compiler hardening is enabled, including PIE and BIND_NOW on the zlib-flate and qpdf binaries (yay!). There are no initscripts/upstart jobs, dbus services, setuid, fcaps usage, sudo/su/pkexec usage or cron jobs.

Doing a shallow code audit:
* there are uses of sprintf, but they are properly bounds checked with the destination being on the stack, so they are protected via stack-protector. The QUtil::copy_string() is well implemented.
 * There were a few strcpy's as well, but they were also safely used and protected by stack-protector.
 * memory allocations are via C++'s 'new' and not C-style *malloc
 * I also verified Jay's statements regarding libqpdf/QPDF.cc

On the whole, from everything I looked at, qpdf is defensively programmed, well maintained and a good candidate for main even though it has 'pdf' in its name. Thanks Michael, Jay and Tobias for the discussion.

ACK