[MIR] openjpeg2

[bbordwell]

openjpeg should be included in main because compiling poppler with --enable-openjpeg in debian/rules gives poppler greater functionality (please see bug 710412). Since this change to /debian/rules adds openjpeg as a build-dep to poppler, which is in main, openjpeg must also be in main.

ImageMagick also needs openjpeg in main so it can be built with JPEG2000 support. (LP: #1447968)

Main inclusion requirements:

1. It is already in universe.

2. The package is a new build-dep and has a large user base (think evince).

3. Searching https://secuniaresearch.flexerasoftware.com/community/advisories/search/ for openjpeg gave zero results.

4. openjpeg has no current Ubuntu bugs (https://bugs.launchpad.net/ubuntu/+source/openjpeg2)

Debian bugs at https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=openjpeg2

openjpeg does not require any configuration or debconf questions.

5. N/A

6. All build-deps are already included in main.

7. I am afraid that this is a bit over my head. Hopefully, someone else could ensure that this package meets the requirements here. Based on its long inclusion in Debian and Ubuntu, I think that it should be alright here.

8. This is a fairly simple program that doesn't need too much maintenance as shown by the bug reports.

9. The package title and description seem to be in order.

[Michael Terry (mterry)]

Didier, got time for this one?

[Didier Roche-Tolomelli (didrocks)]

I reasonably won't have the time to do it unfortunately, if someone can handle it, it would be nice :)

[Matthias Klose (doko)]

- the sources contain an embedded libtiff. is it needed? is it used for the build?

- the libjpeg8 source README reads:

FILE FORMAT WARS
================

The ISO JPEG standards committee actually promotes different formats like
"JPEG 2000" or "JPEG XR" which are incompatible with original DCT-based
JPEG and which are based on faulty technologies. IJG therefore does not
and will not support such momentary mistakes (see REFERENCES).
We have little or no sympathy for the promotion of these formats. Indeed,
one of the original reasons for developing this free software was to help
force convergence on common, interoperable format standards for JPEG files.
Don't use an incompatible file format!
(In any case, our decoder will remain capable of reading existing JPEG
image files indefinitely.)

do we want libopenjpeg2 in main? currently we only have as rdepends of libopenjdk2:

Reverse Depends:
  libavcodec-extra-52
  mupdf
  libgmerlin-avdec1
  gpac
  blender

[Sebastien Bacher (seb128)]

> - the sources contain an embedded libtiff. is it needed? is it used for the build?

the package seems to be using the system version

> do we want libopenjpeg2 in main? currently we only have as rdepends of libopenjdk2:

libopenjdk2? you mean libopenjpeg2? see the rational, builder poppler with it would allow to open pdf using images in that format

[Michael Terry (mterry)]

Yeah, Debian patches this to use the system tiff library. Doko, can you look at this again?

[Matthias Klose (doko)]

> see the rational, builder poppler with it would allow to open pdf using images in that format

see the text I did quote:

"(In any case, our decoder will remain capable of reading existing JPEG
image files indefinitely.)"

or can you find a document/image which isn't properly displayed?

[bbordwell (benbordwell)]

Matthias, as stated in the original bug report there are some images that are not displayed properly. see bug 710412.

I am also uploading the pdf to this bug report.

[Matthias Klose (doko)]

On 07/12/2011 09:43 PM, bbordwell wrote:
> Matthias, as stated in the original bug report there are some images
> that are not displayed properly. see bug 710412.

thanks. was this tested with jpeg62 or jpeg8, or is this still found in jpeg8?

[Launchpad Janitor (janitor)]

[Expired for openjpeg (Ubuntu) because there has been no activity for 60 days.]

[Albert Astals Cid (aacid)]

Ok, so some answers here:

Does libjpeg provide the same openjpeg does?
No, libjpeg is for regular jpeg and openjpeg is for jpeg2000 that has nothing to do with the first one other than the name

Can you provide an image that needs openjpeg?
No, but i can provide a pdf file that poppler needs openjpeg to be linked in to be renderer properly https://bugs.launchpad.net/ubuntu/+source/poppler/+bug/881407/+attachment/2571595/+files/Manning-ExtJS_in_Action.pdf
Actually i had some images openjpeg opens and jasper does not, they are buried somewhere deep down in some kde mailing list if you really need them

Embedded tiff?
As far as i know it's just for windows developers convenience, they are only used if not found on your system. At least when compiling openjpeg 1.5 with cmake i get
-- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.3.4")
-- Your system seems to have a Z lib available, we will use it to generate PNG lib
-- Found PNG: /usr/lib/x86_64-linux-gnu/libpng.so
-- Your system seems to have a PNG lib available, we will use it
-- Found TIFF: /usr/lib/x86_64-linux-gnu/libtiff.so
-- Your system seems to have a TIFF lib available, we will use it
-- Found LCMS2: /usr/lib/x86_64-linux-gnu/liblcms2.so
-- Your system seems to have a LCMS2 lib available, we will use it

We already have jasper in main that is a jpeg2000 library?
Right, but poppler does not have code for jasper and jasper last release seems to be in 2007 and openjpeg is actively developed and released

Anymore thing you guys need answering?

[Jamie Strandboge (jdstrand)]

Should jasper be demoted and openjpeg used instead for things we support?

[Albert Astals Cid (aacid)]

Another note, we are only shipping openjpeg 1.3 while 1.5 is out with lots of crash fixes (i forwarded them lots of fuzzed crashers that crashed in poppler because of libopenjpeg) so it's a good idea to update to this new version too

[Albert Astals Cid (aacid)]

''Should jasper be demoted and openjpeg used instead for things we support?''

No idea how easy is to port from jasper to openjpeg so can't comment on this.

[Jamie Strandboge (jdstrand)]

I have not performed a full review yet, but can say openjpeg does have a security history:
- CVE-2009-5030
- CVE-2012-1499
- CVE-2012-3358

There is an open security issue in openjpeg right now: bug #1023259 (CVE-2012-3358). This will have to be fixed before promoting openjpeg.

[Jamie Strandboge (jdstrand)]

CVE-2012-3358 is now fixed, but in the meantime, CVE-2012-3535 was discovered. This makes 4 CVEs for its history, 2 within the period of when I have gotten involved in the MIR, which is not building confidence in the software. CVE-2012-3535 needs to be fixed before (re)considering for main.

[Albert Astals Cid (aacid)]

openjpeg 1.5.1 released

[Till Kamppeter (till-kamppeter)]

Needed by Ghostscript 9.08.

[Till Kamppeter (till-kamppeter)]

The CVEs are fixed in the current package, it contains patches named cve-2012-3358.dpatch and cve-2012-3535.dpatch.

[Seth Arnold (seth-arnold)]

I reviewed openjpeg 1.3+dfsg-4.6ubuntu2 from saucy. This should not
be considered a full security audit, but rather a quick gauge of code
cleanliness.

- openjpeg provides a library interface and command line utilities for
  manipulating jpeg2000 formatted files.
- build-deps upon libtiff-dev
- Does not use cryptography, does not itself do networking
- Does not daemonize
- Does not provide initscripts
- Does not provide D-Bus services
- Does not provide setuid executables
- Provides four programs
  - index_create
  - jp2-thumbnailer
  - image_to_j2k
  - j2k_to_image
- Does not provide sudo fragments
- Does not provide cron jobs
- Messy build logs, most warnings can be safely ignored but these may be
  serious:
  - signedness error mistakes in j2k_index_JPIP() and one program's main()
  - 'tmp' may be used uninitialized in j2k_read_sot()
- Frequent casting of malloc(3)'s return value defeats compiler warnings
- Incorrect function prototyping defeats compiler warnings
- I did not discover a test suite.

[ Details redacted until 2013-09-09 -- sarnold 2013-08-28 ]
- cio_*() family of routines never check out-of-bounds reads and writes
  before the allocated buffer, even though cursor manipulations frequently
  rewind the cursor. I'm surprised such an obvious reliability measure is
  missing.

I have applied for CVE numbers.

I stopped auditing this package part-way through, so the above list of
problems is not exhaustive. This package needs a severe overhaul.

Security team NAK for promoting to main.

Thanks

[Albert Astals Cid (aacid)]

Why one would review 1.3 instead of 1.5.1 or 2.0 escapes my mind.

[Till Kamppeter (till-kamppeter)]

Looks like that the openjpeg project is dead upstream, leaving us with no maintained free software JPEG2000 support (Jasper already died much earlier). I hope Artifex (upstream of Ghostscript) will kick in here.

If not, anyone volunteering is welcome.

For now we must consider Ghostscript providing its own internal JPEG2000 support.

See IRC chat with GS developers:

<tkamppeter> chrisl, I switched back to built-in openjpeg, as I did not get it working with the system's library (GS simply built without openjpeg, without reporting any error).
<chrisl> tkamppeter: it might fallback to Jasper, or might just disable JPX, I can't remember off the top of my head.
<tkamppeter> chrisl, seems that openjpeg is of a rather bad coding quality: https://bugs.launchpad.net/ubuntu/+source/openjpeg/+bug/711061, especially comment #19. Are the enhancements of the GS developers (which upstream is ignoring/rejecting?) fixing these problems?
<kens> tkamppeter : the 2.0 patches have not yet been submitted upstream I was incorrect in that. The developer is waiting until we adopt them ourselves I believe (at least the ones relating to GS code anyway)
<kens> I think a lot of the criticisms in comment #19 are irrelvant to its use in Ghostscript
<chrisl> kens, tkamppeter: actually, he's waiting for responses (of any kind!) from the openjpeg devs..... they've gone dark again :-(
<kens> tkamppeter : when it comes to JPEG2000 support we have a choice of JasPer or OpenJPEG. JasPer is terribly badly written, does not support all features, and is no longer maintained. OpenJPEG may not be ideal, but we htink its better than JasPer on all points.
<chrisl> tkamppeter: henrys has already suggested we "really" fork openjpeg for our use, since the project seems decidedly moribund now :-(
kens is disappointed to hear that
<chrisl> Well, I think Shelly's been waiting ~3 weeks for a reply to a simple question, and not even had an acknowledgement from the dev team
<kens> Great, 2 open source implementations, both dead :-(
<chrisl> I still have the feeling that OPJPG devs had their own requirements, and once those were met, they've backed off. I could wrong though
<tkamppeter> chrisl, so I think, to get solid JPEG2000 support into Linux/free software it would be really the best if Artifex forks libopenjpeg and makes it available as separate free software project.
<kens> Umm, I'm not sure we have the resources to support a 'proper' OpenJPEG library
<chrisl> tkamppeter: the trouble is it's a *lot* of work.....
<chrisl> tkamppeter: we might be able to "manage" the project, host the repo etc, but the development load would probably be more that we could handle

[Albert Astals Cid (aacid)]

http://code.google.com/p/openjpeg/source/list shows some development, not billion of things, but something.

[Sebastien Bacher (seb128)]

@Albert: the security team reviews what is in the Ubuntu archive, since that's what would be promoted/shipped to users. I guess we need to update that library next cycle

[Seth Arnold (seth-arnold)]

Albert, when I reported vulnerabilities to the developers and other distribution vendors, I tracked down many of the issues in the openjpeg trunk/ branch in subversion. The worst of the offenses have not yet been corrected in newer releases.

If you care about the openjpeg codebase and want to see its wider adoption, please consider getting a Coverity account to perform a detailed and thorough analysis. I only took a day or so of time to review code and reviewed by hand. Probably the Coverity team can point out far more issues to correct than I can.

[Till Kamppeter (till-kamppeter)]

Can this be re-checked on the current version (1.5.2)? Debian has this version in Main and it is synced to Ubuntu. Debian is using this version for Ghostscript 9.15 which I have synced into Ubuntu so that Ubuntu has the current Ghostscript and minimum delta to the Debian package.

[Till Kamppeter (till-kamppeter)]

Can this please get re-checked. Debian is using openjpeg for their Ghostscript package and as Debian acknowledged the new license of Ghostscript (AGPL) we could get back to syncing Ghostscript with Debian again, bu this will only work if we have libopenjpeg in Main.

[Michael Terry (mterry)]

Based on comments 19 and 24, even version 1.5.2 would presumably not pass a security review. In Seth's words, "This package needs a severe overhaul."

Til, you even say in comment 21 that openjpeg is dead upstream. I get that it might be reviving (comment 22), but unless you say otherwise, I will assume that a point release is not where a severe overhaul has taken place.

I understand the frustration with the lack of proper jpeg2000 support. But I'd rather not promote a package we believe will just create security problems down the road.

[Till Kamppeter (till-kamppeter)]

I told in August 2013 that it seems dead upstream. I have no idea what is the situation today. I was simply hoping that it got better in the last two years and seeing that Debian, a distro very much oriented in stability and server use, using it in Ghostscript it made the impression for me that it improved. Therefore I asked for revisiting this MIR.

If the state of libopenjpeg is still as bad as before it is no problem for me to continue Ghostscript separate from Debian. perhaps also assuming that the Ghostscript upstream developers are more into security and therefore their copy of libopenjpeg in the Ghostscript source is better than the original (see comment #21).

Also no one complained about the JPEG2000 support in our Ghostscript (using the openjpeg copy with comes with Ghostscript) and also no security bug reports related to this appeared. This can mean that the built-in openjpeg is "good enough" for Ghostscript and has the vulnerable parts not used or fixed by Ghostscript developers.

In addition, JPEG2000 seems an exotic format for me which did not really get adopted, it exists for years and I have owned several digital cameras (including DSLR and mirrorless cameras) since 2001 and they all do classic JPEG and RAW, JPEG2000 never made it into a camera. Where is JPEG2000 actually used?

[Albert Astals Cid (aacid)]

openjpeg has never been dead https://github.com/uclouvain/openjpeg/commits/master shows it has had continuous commits for a long time

JPEG2000 is "quite" used in PDF files.

Now the security claim is weird to me since we have stuff in main like poppler that has a less secure jpeg2000 decoder than openjpeg, but oh well, i won't claim to understand security :D

[Till Kamppeter (till-kamppeter)]

Note also that there is actually no alternative for openjpeg. It is the only actively developed JPEG2000 library. If it turns out that it is still a high security risk and it is important to have the Ghostscript upstream developers as an extra security instance to integrate it into Ghostscript for us and not let app developers freely use it and not trust Debian who have it in Main, we have to keep Ghostscript different from Debian, using the Ghostscript-embedded openjpeg.

I think we never can do completely away with JPEG2000, as this makes several PDFs not displaying/printing and me receiving a lot of bug reports. As JPEG2000 is not used standalone but only in PDF we can continue using Ghostscript with embedded openjpeg, my main intentions to use openjpeg as separate library are:

- Syncing GS with Debian and so ease maintainership
- Getting MuPDF into Main which probably also uses openjpeg

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Robert Ancell (robert-ancell)]

poppler 0.38.0 now stated on configure:
"Warning: Using libopenjpeg is recommended. The internal JPX decoder is unmaintained."

[Till Kamppeter (till-kamppeter)]

For being able to sync the Ghostscript package with Debian we still need this MIR (all other MIRs needed for Ghostscript are done).

[Till Kamppeter (till-kamppeter)]

openjpeg needed by both Poppler and Ghostscript makes it even more convenient to have this library as separate package in the system instead of letting Ghostscript and Poppler use their own embedded versions, especially for system security and maintainability.

[Till Kamppeter (till-kamppeter)]

Any news on this one?

[Seth Arnold (seth-arnold)]

Probably this won't be investigated until mid or late January.

Thanks

[Till Kamppeter (till-kamppeter)]

We reached mid or late January now, any chance to get this solved for 16.04?

[Till Kamppeter (till-kamppeter)]

Any chance to get this done before Feature Freeze for 16.04?

[Tyler Hicks (tyhicks)]

On 2016-02-04 15:39:13, Till Kamppeter wrote:
> Any chance to get this done before Feature Freeze for 16.04?

It isn't likely to happen by feature freeze but it should be done by
16.04.

[Will Cooke (willcooke)]

Just to reiterate, this is vital for us in 16.04. Please let me know what we can do to help make sure this happens.

[Seth Arnold (seth-arnold)]

I reviewed openjpeg version 1:1.5.2-3.1 as checked into Xenial. This
should not be considered a full security audit.

Security team NAK for moving openjpeg to main for 16.04.

The source is nearly unreadable due to terrible formatting. I could not
find any tab-sizes that allowed the code to be legible by any reasonable
standard. I strongly recommend upstream should (a) run the entire codebase
through indent(1) or a similar tool (b) enforce a standard, any standard.
K&R recommended but really anything beats what this currently has.

In one day of fuzzing with my laptop I found eight crashers that caused
segmentation violation faults. They may be read-only or perhaps they may
allow remote code execution but the source code is so difficult to read
that I cannot volunteer my time to discover the causes.

cppcheck as packaged with 14.04 LTS issued 21 warnings; almost all
represent real bugs in the package.

Upstream's 2.1.0 release is a mixed bag -- cppcheck reported 61 warnings;
many looked similar to issues from 1.5.2. However, the eight crashing
inputs I found no longer crashed the library. Less than four hours of
fuzzing on my laptop found an input that crashes.

Memory management is sloppy and may in fact provide routes for exploits.

Integer types seem very casual; this library was not written with a
strong awareness of C's extremely dangerous behaviours around signed
integers, the usual integer promotion rules, and sign extension rules.

In addition, this looks like a poor library interface -- errors are sent
directly to stderr without any attempts to offer client programs any
mediation, the error messages never include strerror() reasons meaning
users have no feedback on what failed.

We cannot support this library as it stands. Before we can support it,
the source code layout needs to be addressed, cppcheck should be clean,
and gcc warnings should be addressed. I strongly recommend sending this
through Coverity as well.

Here's a more concrete list of issues I found; I hope these are useful:

- applications/codec/j2k_dump.c allocates a hugely oversized block of
  memory for getting directory listings. If the dirptr or dirptr->filename
  memory allocations fail, the program dies via NULL pointer write errors.
  This should instead work on one file at a time and avoid the entire
  disaster.
- applications/codec/j2k_dump.c parameters.outfile misleading error
  message, the fopen() was for writing, not reading
- applications/JavaOpenJPEG/JavaOpenJPEGDecoder.c get_num_images(),
  load_images() leak dir
- Java_org_openJpeg_OpenJPEGJavaDecoder_internalDecodeJ2KtoImage() fails
  to close 'fsrc' in this error path: if(src == NULL) goto fin
- OPJMarkerTree::OnSelChanged() allocates buffer with new[] but
  deallocates with delete
- bmptoimage() never checks getc() error return code
- bmptoimage() leaks IN
- imagetopgx() leaks name via if (!fdest) error -- memory handling here is
  far too complicated
- applications/codec/image_to_j2k.c main() far too complicated
- applications/codec/image_to_j2k.c main() leaks f (not really relevant
  since the 'return 1' exits, but once this code is broken apart into
  something legible this will matter more)
- yu...

[Seth Arnold (seth-arnold)]

https://github.com/uclouvain/openjpeg/issues/718 -- fuzz-generated inputs
https://github.com/uclouvain/openjpeg/issues/719 -- cppcheck issues

[Till Kamppeter (till-kamppeter)]

Can someone of the security people have a look at this fork of libopenjpeg by the Ghostscript upstreams:

http://git.ghostscript.com/?p=thirdparty/openjpeg.git;a=summary

I am looking into whether this could make up a replacement of libopenjpeg in Ubuntu, at least for the PDF interpreters (Ghostscript, Poppler, in the future also MuPDF).

[Michael Terry (mterry)]

The last commit in that repo is from two years ago. Is there any reason to believe they've fixed the issues Seth found?

[Albert Astals Cid (aacid)]

Other point of view is, poppler uses its own JPEG2000 parser if openjpeg is not present.

That parser is probably worse security wise than the openjpeg one and the poppler developers just keep it for compatibility, but won't refuse to spend time on it when there's maintained code out there that implements JPEG2000 parsing better.

So maybe by makig openjpeg not go to main you're exposing your users to an even bigger threat

[Till Kamppeter (till-kamppeter)]

Michael they use more or less 2.1.0 which is actually 2 years old.

[Michael Terry (mterry)]

Albert, I get what you're saying. But there's a big difference between Ubuntu putting a library in main and a pdf library embedding a copy of that library.

If we put a library in main, it means other packages may start depending on it (and ones that already do can enter main easier). And app developers may depend on it more, since we are promising to officially support it.

Whereas an embedded copy inside a pdf library inherently has a smaller security surface. It's only used for a certain purpose. While pdfs are certainly widely used, they are less widely used than images.

Although, the fact that poppler is shipping copies of unmaintained code is not great either. And we probably shouldn't be enabling poppler's jpeg2000 support if poppler upstream isn't even maintaining its own copy well. That's just sneaking a burden onto the security team.

The security team is already on the hook for one jpeg2000 parser in main (jasper). It's used by gimp, libraw, and gegl (among some other consumers in universe). While jasper's certainly a dead library, the other jpeg2000 options don't seem much better either. Jasper doesn't seem to have ever had a MIR, so it must be grandfathered in from early days.

Given the security team's NAK for openjpeg, the best way forward for jpeg2000 support in poppler would be to port poppler to jasper. That wouldn't need a MIR and would reduce our existing security surface.

I know it's been said in this MIR that jasper is missing some features (or can't handle some images that openjpeg can). Which is a bummer, agreed.

[Seth Arnold (seth-arnold)]

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

[Albert Astals Cid (aacid)]

> Albert, I get what you're saying. But there's a big difference between Ubuntu putting a library in main and a pdf library
> embedding a copy of that library.

It's not a copy of the library but self grown code

> You raise the possibility that poppler's existing code may be worse; if so, we should probably disable it before Xenial
> is released.

lol, talk about boomerang effect :D

[Till Kamppeter (till-kamppeter)]

Moving on this MIR to openjpeg2.

[Jeremy Bícha (jbicha)]

jasper will be removed from Debian soon. I think the only thing currently using jasper in main is imagemagick, see bug 1612822.

Since imagemagick already supports openjpeg2 and actually doesn't support jasper any more, it might be nice if openjpeg2 could simply take jasper's place as jasper is demoted and removed in yakkety.

[Michael Terry (mterry)]

Seth, back to you. I don't know how different a codebase openjpeg2 is from openjpeg. But version numbers got bumped at least. :)

[Seth Arnold (seth-arnold)]

I ran afl-fuzz against the upstream openjpeg 2.1.1 release and found the following corpus of crashing inputs:

68ae4c0f26ff70a7cac6495c430db7e9c42c5a33d81026cfbe0576026556d7f0 crashes-openjpeg-2.1.1.tar.gz

Thanks

[Seth Arnold (seth-arnold)]

I've filed https://github.com/uclouvain/openjpeg/issues/811 to ask the OpenJPEG team to look at the 646 crashing inputs uncovered by AFL. (Sorry about the extra messages, but github won't let me upload attachments. So launchpad is most convenient for hosting the tarball.)

Thanks

[Mathew Hodson (mhodson)]

ImageMagick also needs openjpeg in main so it can be built with JPEG2000 support. (LP: #1447968)

[Seth Arnold (seth-arnold)]

Indeed it might be worth another look; there has been upstream activity addressing issues and the commit messages even reference Coverity. They've been trying.

If jpeg2000 support in Ubuntu is important to you, I'd like to encourage you to:
- read the openjpeg2 source code and suggest improvements
- contribute images to a test suite
- contribute a test suite if that's still lacking
- run coverity or cppcheck or other static analysis tools on the codebase
- fuzz the library with ubsan, asan, libdislocator, etc.
- write libfuzzer-friendly wrappers around the functions and contribute these to the project if that's still lacking.

Something really simple would be to build an asan or libdislocator variant and test with the files I've already generated and attached here. If any of the files I've attached in the last year still cause asan alerts when I get around to looking at the library again, it'll be a pretty poor report on the state of the library.

Thanks

[Bryan Quigley (bryanquigley)]

I've found a regression [1]in Poppler 17.10 (worked fine in 17.04) that getting this in main would solve. I'm still not parsing exactly why this has regressed, but building with openjpeg2 support did fix it.

[1] https://bugs.launchpad.net/ubuntu/+source/poppler/+bug/1714596

[Seth Arnold (seth-arnold)]

I've started in on a new review of openjpeg2. The code is vastly improved since the last time I read it but it still has rough edges. So far I've filed:

https://github.com/uclouvain/openjpeg/issues/1065
https://github.com/uclouvain/openjpeg/issues/1066
https://github.com/uclouvain/openjpeg/issues/1067
https://github.com/uclouvain/openjpeg/issues/1068
https://github.com/uclouvain/openjpeg/issues/1069
https://github.com/uclouvain/openjpeg/issues/1070
https://github.com/uclouvain/openjpeg/issues/1071
https://github.com/uclouvain/openjpeg/issues/1072
https://github.com/uclouvain/openjpeg/issues/1073
https://github.com/uclouvain/openjpeg/issues/1074
https://github.com/uclouvain/openjpeg/issues/1075
https://github.com/uclouvain/openjpeg/issues/1076
https://github.com/uclouvain/openjpeg/issues/1077
https://github.com/uclouvain/openjpeg/issues/1078
https://github.com/uclouvain/openjpeg/issues/1079

cppcheck still returns a lot of results, it'd be nice to bring that to zero.

Thanks

[Seth Arnold (seth-arnold)]

I've filed:

https://github.com/uclouvain/openjpeg/issues/1082
https://github.com/uclouvain/openjpeg/issues/1083
https://github.com/uclouvain/openjpeg/issues/1084
https://github.com/uclouvain/openjpeg/issues/1085
https://github.com/uclouvain/openjpeg/issues/1086
https://github.com/uclouvain/openjpeg/issues/1087
https://github.com/uclouvain/openjpeg/issues/1088
https://github.com/uclouvain/openjpeg/issues/1089

I'll move on to other MIRs for a while and let the upstream folks get a handle on these.

Thanks

[Misaki (myjunkmail311006)]

Regarding security: it seems that ffmpeg has retained jpeg-2000 support during this time. ffmpeg's configuration,

ffmpeg version 3.4.2-2 Copyright (c) 2000-2018 the FFmpeg developers
  built with gcc 7 (Ubuntu 7.3.0-16ubuntu2)

[...]
--enable-libopenjpeg
[...]

ffplay will display a jpeg2000 image, although I get a lot of warnings about 'End mismatch <number>'.

Is ffmpeg not exposed to any potential security flaws that imagemagick would be?

[Seth Arnold (seth-arnold)]

Hi Misaki,

There's multiple interacting issues:

- ffmpeg is in universe; thus, many sites will not install it because they configure apt to only install packages from main.

- imagemagick's insanely useful tools are used by hundreds or thousands of other applications.

- openjpeg's upstream developers have made really impressive progress improving their code quality but it still appears to be a hobby / part time project rather than a production ready tool.

At this point I'd probably even say openjpeg's quality is slightly better than imagemagick's quality. imagemagick is included in main because the effort to *remove* it from main would be substantial. Were imagemagick to be proposed as a new addition today it would not meet our quality expectations.

However, I'm confident that at least some of the issues I've raised with openjpeg would allow for remote zero-interaction exploits of our desktop users if its code were properly exposed. It could be via attached images in emails being automatically thumbnailed, downloaded documents being automatically thumbnailed, etc. Perhaps album artwork on streaming music services. Probably not everything I've found is actually exploitable but I've flagged so many potential issues that it's entirely likely there's multiple paths to exploitation.

The openjpeg team has come so far, it'd be a shame if they didn't cross the finish line at this point. (I also hope the imagemagick team can make similar strides, but hopefully everyone knows to run imagemagick commands in AppArmor profiles or SELinux policy by now.)

Thanks

[Matthias Klose (doko)]

setting to incomplete again, based on the review above.

[Michael Catanzaro (mike-catanzaro)]

The security review in comment #59 and comment #60 looks very nice. I skimmed over the issues and noticed that almost all of them affect the utility tools (in bin), not the library itself. You may or may not consider that relevant to the MIR. The issues affecting the library code are:

https://github.com/uclouvain/openjpeg/issues/719
https://github.com/uclouvain/openjpeg/issues/1071
https://github.com/uclouvain/openjpeg/issues/1076
https://github.com/uclouvain/openjpeg/issues/1077
https://github.com/uclouvain/openjpeg/issues/1078

There's also issue 1073, but that issue is disputed by upstream and doesn't affect Ubuntu anyway because Ubuntu uses system libtiff instead of the bundled code. All the other issues are in bin.

[Michael Catanzaro (mike-catanzaro)]

Even better: #1077 can be immediately closed as a duplicate of #1078 (which contains discussion), and then you already fixed #1071 and just forgot to close. So that leaves us with two specific security issues affecting the library, #1076 and #1078, plus the "make cppcheck happy" issue #719.

[Michael Catanzaro (mike-catanzaro)]

Actually, both #1076 and #1078 are in the mj2 library, which Ubuntu disables with the -DBUILD_MJ2:BOOL=OFF CMake arg. Additionally, all of the cppcheck issues in #719 that are not under bin are in this mj2 library, except for one:

[lib/openjpip/j2kheader_manager.c:120]: (error) Uninitialized variable: COD

So I believe this is the only issue flagged by Seth that actually affects OpenJPEG when used as a library (and with mj2 library disabled).

Of course, the issues with the tools and mj2 library are not encouraging, but the situation doesn't seem as bad as I had thought.

[Seth Arnold (seth-arnold)]

Hello Michael, thanks for giving this a new look. I know enough people have interest in working with JPEG2000 files -- this is a frequent request. The OpenJPEG team has really done a lot of work to improve the library, and it'd be well and truly satisfying to be able to move it to main.

I'd really love for https://github.com/uclouvain/openjpeg/issues/1079 to be addressed. The *fuzzer* has implemented safety checks that should have been implemented in the library. Simply moving these checks from the fuzzer to the library would have a huge impact on safety and make the fuzzer far more effective.

Of course, this only pays dividends if someone is fixing issues as ossfuzz finds and reports them.

Thanks

[Michael Catanzaro (mike-catanzaro)]

Hm that makes sense!

From my reading of that issue, it's clear that you want the checks removed from the fuzzer, but not so clear that you want them added to the main library. That might be worth clarifying with upstream.

[jonathan green (jonathangreen)]

It looks like https://github.com/uclouvain/openjpeg/issues/1079 was recently resolved, which hopefully can help to move this issue forward!

[Matthias Klose (doko)]

it was noted that img2pdf ftbfs with an JPEG2000 test error in
https://launchpad.net/ubuntu/+source/img2pdf/0.3.3-1

Maybe it's worth finding out why

[Eduardo Barretto (ebarretto)]

I reviewed openjpeg2 2.3.1-1 as checked into focal. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

openjpeg2 is a library to encode and decode JPEG 2000 images. JPEG 2000 is an
image compression standard and coding system. OpenJPEG dates back from 2005
and has become the JPEG 2000 reference software in 2015.

- CVE History:
  - openjpeg has been assigned CVEs every year since 2012. For Xenial we still
    have some 2016 CVEs that we are unaware of the fix. There are also a couple
    of CVEs that don't have fix or we are unsure if they were solved:
    CVE-2018-16376, CVE-2018-20846, CVE-2019-6988
  - Upstream is responsive and willing to fix security issues, but they still
    need to improve on how to communicate about the fixes.
- Build-Depends:
  - cmake
  - debhelper
  - default-jdk
  - dh-apache2
  - help2man
  - javahelper
  - libcurl4-gnutls-dev or libcurl-ssl-dev
  - libfcgi-dev
  - liblcms2-dev
  - libpng-dev
  - libtiff-dev
  - libxerces2-java
  - zlib1g-dev
- postinst, prerm and postrm scripts automatically added
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- binaries in PATH
  - /usr/bin/opj_compress - This program reads in an image of a certain type
    and converts it to a JPEG2000 file.
  - /usr/bin/opj_decompress - This program reads in a JPEG2000 image and
    converts it to another image type.
  - /usr/bin/opj_dump - This program reads in a JPEG2000 image and dumps the
    contents to stdout.
  - /usr/bin/opj_jp3d_compress - compress into JP3D volume.
  - /usr/bin/opj_jp3d_decompress - decompress JP3D volume.
  - /usr/bin/opj_dec_server - server to decode JPT/JPP-stream and communicate
    locally with JPIP client, which is coded in java.
  - /usr/bin/opj_jpip_addxml - embed metadata into JP2 file.
  - /usr/bin/opj_jpip_test - test index code format of a JP2 file.
  - /usr/bin/opj_jpip_transcode - convert JPT/JPP-stream to JP2 or J2K.
  - /usr/bin/opj_server - JPIP server supporting HTTP connection and
    JPT/JPP-stream.
  - /usr/bin/opj_jpip_viewer
- No sudo fragments
- No udev rules
- openjpeg2 has 1478 tests under tests/, including Google's oss-fuzzers setup.
  - some of those tests are CVEs reproducers.
- No cron jobs
- Build logs:
  - Multiple compiler warnings:
/<<PKGBUILDDIR>>/src/lib/openjp2/openjpeg.c:1041:30: warning: cast between incompatible function types from int (*)(FILE *) {aka int (*)(struct _IO_FILE *)} to void (*)(void *) [-Wcast-function-type]
/<<PKGBUILDDIR>>/src/bin/jp3d/opj_jp3d_decompress.c:488:5: warning: ignoring return value of fread, declared with attribute warn_unused_result [-Wunused-result]
/<<PKGBUILDDIR>>/src/bin/jp3d/convert.c:111:5: warning: ignoring return value of fread, declared with attribute warn_unused_result [-Wunused-result]
/<<PKGBUILDDIR>>/src/bin/jp3d/convert.c:118:5: warning: ignoring return value of fread, declared with attribute warn_unused_result [-Wunused-result]
/<<PKGBUILDDIR>>/src/bin/jp3d/convert.c:119:5: warning: ignoring return value of fread, declared with attribute warn_unused_result [-Wunused-result]
/<<PKGBUILDDIR>>/src/bin/jp3d/convert.c:130:5: warning: ignoring return value of fread, decla...

[Christian Ehrhardt  (paelzer)]

FYI this still lacks a team subscriber - per the former comments I'd have expected "desktop-packages" but haven't found that one.

[Christian Ehrhardt  (paelzer)]

With above analysis done, in conjunction with the decisions in Paris and per the discussion in the MIR team meeting at [1] this is an ack.

Please go forward with vendored dependencies, that applies to:
1. the security team which has this on its queue for review
2. the server team for an eventual upload

http://ubottu.com/meetingology/logs/ubuntu-meeting/2020/ubuntu-meeting.2020-01-14-14.03.moin.txt

[Christian Ehrhardt  (paelzer)]

This already had the Security review acked - thanks ebarretto for clarifying.
The only thing missing was a Team subscriber.

$ ./get-packages-subscribed.py --team desktop-packages -p | grep openjpeg
openjpeg2

The missing subscription is now resolved, therefore this is ready.
It is not yet in component mismatches, so per [1] I'll mark it "In Progress" and unassign the security Team.

@Desktop - are you adding a dependency or seed change to pull it in?

[1]: https://wiki.ubuntu.com/MIRTeam#Process_states

[Christian Ehrhardt  (paelzer)]

@didrocks, please forgive me but to avoid this being lost I assigned you for now - feel free to re-assign inside the team as needed.

[Till Kamppeter (till-kamppeter)]

Great, finally succeeded after 9 (!) years!

I will soon update the Ghostscript packages, merging 9.50 from Debian and switch over to use the libopenjpeg2 instead of the Ghostscript-internal library.

Other target is Poppler, I hope the Poppler package maintainer is aware.

[Didier Roche-Tolomelli (didrocks)]

No worry! I'll promote it once we have something pulling it in the archive

[Till Kamppeter (till-kamppeter)]

synced Ghostscript 9.50 from Debian, pulling in libopenjpeg2.

[Didier Roche-Tolomelli (didrocks)]

Promoted

$ ./change-override -c main -t openjpeg2
Override component to main
openjpeg2 2.3.1-1 in focal: universe/misc -> main
Override [y|N]? y
1 publication overridden.
$ ./change-override -c main libopenjp2-7
Override component to main
libopenjp2-7 2.3.1-1 in focal amd64: universe/libs/extra/100% -> main
libopenjp2-7 2.3.1-1 in focal arm64: universe/libs/extra/100% -> main
libopenjp2-7 2.3.1-1 in focal armhf: universe/libs/extra/100% -> main
libopenjp2-7 2.3.1-1 in focal i386: universe/libs/extra/100% -> main
libopenjp2-7 2.3.1-1 in focal ppc64el: universe/libs/extra/100% -> main
libopenjp2-7 2.3.1-1 in focal s390x: universe/libs/extra/100% -> main
Override [y|N]? y
6 publications overridden.