Albert, my subjective assessment of the openjpeg code is that it is a
very long way from the quality people expect from Ubuntu.

If you have the time and inclination to help the openjpeg team, please
do. I suggest:

- Re-indent the entire codebase. What I reviewed was illegible. (The
Linux kernel's Lindent script is tasteful but I've heard good things
about clang's formatter.)

- Fix all issues reported by cppcheck.

- Fix all issues reported by gcc warnings. (I recently reviewed another
package that used -Wdate-time -Wall -W -Wshadow -Wstrict-prototypes
-Wpointer-arith -Wcast-align -Wno-strict-aliasing -Wwrite-strings
-Wformat -Werror=format-security. openjpeg is currently using only
-Wformat -Werror=format-security -Wdate-time.)

- Remove every instance of signed integers in the code. (Probably no
valid uses of signed integers exist here. Be sure you can justify the
ones that survive.)

- Break down many functions into multiple smaller component functions
that do one thing. There's many functions that logically should be five
to ten functions instead. (A good first approximation would be to find
every function that's more than 30 lines long and figure out how to
break it apart.)

- Fix all issues reported by running the test suite with a version
compiled with UBSAN and ASAN.

- Fix all issues reported by running AFL with a version compiled with
UBSAN or potentially a 32-bit build with ASAN. While there's a good
chance AFL will find issues with nearly every code base, our openjpeg
packages had half-dozen issues with a few hours of fuzzing.

- Fix issues reported by Coverity.

Image processing libraries are a very popular exploit vector: finding
flaws is easy for attackers (fuzzing these types of programs is far
easier than e.g. network protocols) and image libraries accept input
from a huge variety of sources. Finding flaws in one may give an
attacker full control over a huge number of programs.

That's why I'm being firm on openjpeg. It will be on the front-lines of
our users' safety and it fails badly on the simple things: compile
without warnings, don't have obvious bugs discoverable with simple (and
free!) static analyzers. Stand up to a casual fuzzing effort. Look like
it's been designed well enough that someone not familiar with the
project could fix bugs.

You raise the possibility that poppler's existing code may be worse; if
so, we should probably disable it before Xenial is released. Ideally,
it'd be inspected and if it looks as bad as you make it sound, then
probably we should instigate a switch to jasper. (Not that jasper is
necessarily better; it is effectively abandonware. But we've already
brought it on board for better or worse...)

Till asked about ghostscript's fork of openjpeg. I gave the most recent
dozen commits a quick glance. Those commits looked like they used
standardized formatting, so the initial impression is very good.
However, it's been two years since they've made any commits and I'd be
surprised if they already fixed all the bugs that the openjpeg team have
fixed in the last two years. Any better assessment would require a
significant time investment. cppcheck only found one issue, which is
promising.

Thanks

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/711061

Title:
  [MIR] openjpeg

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/openjpeg/+bug/711061/+subscriptions

-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to