[seth.arnold () canonical com: CVE Requests openjpeg]

[Seth Arnold <seth.arnold () canonical com>]

Please find attached three mails relating to the openjpeg
(http://www.openjpeg.org/) libopenjpeg library (Debian/Ubuntu source
package name openjpeg) JPEG 2000 image processing library.

CVE-2013-4289 has been assigned for heap-based buffer overflows.
CVE-2013-4290 has been assigned for stack-based buffer overflows.

When I reviewed our packages, I followed several of the memory-allocation
multiplications far enough to convince myself that integer wraparound
was possible, though perhaps the example I selected for the CVE request
is not an actual problem.

I have also attached an email I sent directly to the openjpeg developers
with suggestions for further enhancements that did not feel worty of a CVE
request. I'm including it here in the hopes that it is useful to someone.

I did not receive any replies from the openjpeg developers; as far as I
know, no patches are available, and I personally will not be taking the
time to prepare patches.

If someone does take the time to prepare patches, I'd like to suggest
writing several wrappers around malloc() to handle the common memory
allocation cases so the size validation can be performed in a handful of
routines rather than scattered through the codebase.

Thanks to Huzaifa Sidhpurwala for his assistence deciphering my request.

Thanks,
Seth

--- Begin Message --- From: Seth Arnold <seth.arnold () canonical com>
Date: Wed, 28 Aug 2013 21:24:53 -0700

Hello,

I performed a very quick audit of openjpeg recently and discovered
multiple flaws that I believe require CVE numbers.

Please keep linux-distros () vs openwall org[1] CC'd for any updates on
this issue.

This issue is embargoed and has not been disclosed publicly. We are
requesting a coordinated release date (CRD) of 2013-09-09 16:00 UTC.
We ask that you keep this issue embargoed until the CRD[2]. If you or
members of linux-distros () vs openwall org do not request another date,
Ubuntu will make this bug public on the CRD.

I'm asking secalert () redhat com to assign several CVE numbers for these
issues. Please include these CVE numbers in any changelogs.

Thanks in advance for your cooperation in coordinating a fix for this
issue,


- Many instances of malloc() and opj_malloc() using integers multiplied
  together or added together without any overflow checks:
  e.g.
  http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/jp3d.c#1825
  http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/jp3d.c#487

- Several incorrect uses of strncpy() with data that may not have a NUL
  terminating byte within the indicated space:
  e.g.
  http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/opj_jp3d_compress.c#260
  http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/opj_jp3d_compress.c#279

- Several incorect uses of strcpy() with data that may be longer than
  expected:
  e.g.
  http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/convert.c#188
  http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/convert.c#192

- Several incorrect uses of strcat() before accounting for the lengths:
  e.g.
  http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/event.c#118
  http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/event.c#132

- An incorrect use of sprintf() which can overflow a stack-based buffer:
  http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/event.c#158

  Because I found this last one very interesting, I'll paste it here:

        case 'f' :      /* floats */
        {
                char tmp[16];
                double value = va_arg(arg, double);
                sprintf(tmp, "%f", value);
                strcat(message, tmp);
                j += strlen(tmp);
                ++i;
                break;
        }

  Guess how much space "%f" can take up in an sprintf(3): 317 chars(!)

  #include <stdio.h>
  #include <float.h>
  
  int main(void) {
          printf("%f", -DBL_MAX);
  }

  $ ./float_length | wc -c
  317

Some problems are from command-line paramters. Some problems are from
image-supplied parameters. I believe even the command-line based problems
need CVEs because these tools may be used as part of a web-based image
processing mechanism with untrusted users potentially supplying filenames.

These links are far from exhaustive -- they were just the ones I happened
to easily re-find in the newer codebase. I hope the maintainers have the
time and ability to inspect the entire codebase when preparing patches.

I believe this needs at least three, maybe four, CVEs:

- Integer overflows when allocating memory
- Stack-based buffer overflows
- Heap-based buffer overflows
- Failing to NUL terminate strings copied with strncpy()

Thanks,
Seth


[1] linux-distros () vs openwall org is a private mailing list for
distributors of operating systems to collaborate on security
vulnerabilities and coordinate security updates.  Discussions on
linux-distros () vs openwall org are considered private and should not be
made public, though the result of the discussions may be made public
after the coordinated release date.

[2] Please do not release a fix, make public revision control commits,
comment in public bug reports or otherwise disclose information about
this issue until the coordinated release date. This gives all affected
parties a chance to release a fix at the same time.

Attachment: signature.asc
Description: Digital signature

--- End Message ---

--- Begin Message --- From: Seth Arnold <seth.arnold () canonical com>
Date: Wed, 28 Aug 2013 21:44:12 -0700

Hello, I recently gave our openjpeg package a very quick audit, and have
some suggestions for improvements:

- 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

- 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 hope these observations are useful to you.

Thanks,
Seth

Attachment: signature.asc
Description: Digital signature

--- End Message ---

> --- Begin Message ---
>
>
> From: Red Hat Security Response Team <secalert () redhat com>
>
> Date: Thu, 29 Aug 2013 01:39:11 -0400
>
> On Thu Aug 29 00:25:05 2013, seth.arnold () canonical com wrote:
> > Hello,
> >
> > I performed a very quick audit of openjpeg recently and discovered
> > multiple flaws that I believe require CVE numbers.
> Hi Seth,
>
> Thanks for looking at openjpeg, i did some fuzzing some months back
> and found certain important issues myself. I believe that someone needs
> to do a detailed review of openjpeg, not sure if upstream is interested though
> :)
>
>
> > Please keep linux-distros () vs openwall org[1] CC'd for any updates on
> > this issue.
> >
> > This issue is embargoed and has not been disclosed publicly. We are
> > requesting a coordinated release date (CRD) of 2013-09-09 16:00 UTC.
> > We ask that you keep this issue embargoed until the CRD[2]. If you or
> > members of linux-distros () vs openwall org do not request another date,
> > Ubuntu will make this bug public on the CRD.
> >
> > I'm asking secalert () redhat com to assign several CVE numbers for these
> > issues. Please include these CVE numbers in any changelogs.
> >
> > Thanks in advance for your cooperation in coordinating a fix for this
> > issue,
>
> I had a look at one of the issues which you have listed here, and i am not
> really sure if its exploitable, mainly due to my limited knowledge of the
> library. I will anyways assume that you had a detailed look and will
> assign CVE ids.
>
> To give you an example:
> >
> http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/jp3d.c#1825
>
> In this case:
>
> parameters->tcp_numlayers: Though this is defined as an int, only 2 bytes are
> read from the file header and stored in it.
> lib/openjp3d/jp3d.c: tcp->numlayers = cio_read(cio, 2);
>
> Similarly parameters->tcp_numresolution[0] though defined as int, is again
> only 1 byte long as seen in:
>
> lib/openjp3d/jp3d.c: tccp->numresolution[0] = cio_read(cio, 1) + 1;
>
> since array_size is size_t big, i dont think an overflow will happen.
>
> But then again i dont know the codebase :)
>
> So i think two CVEs should be assigned to the issues basically:
>
> > - Many instances of malloc() and opj_malloc() using integers
> > multiplied
> > together or added together without any overflow checks:
> > e.g.
> http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/jp3d.c#1825
> >
> http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/jp3d.c#487
> >
> The above and similar flaws, are integer overflow causing heap-based buffer
> overflow
>
> Please use CVE-2013-4289 for these issues.
>
> > - Several incorrect uses of strncpy() with data that may not have a
> > NUL
> > terminating byte within the indicated space:
> > e.g.
> http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/opj_jp3d_compress.c#260
> >
> http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/opj_jp3d_compress.c#279
> > - Several incorect uses of strcpy() with data that may be longer than
> > expected:
> > e.g.
> http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/convert.c#188
> >
> http://code.google.com/p/openjpeg/source/browse/trunk/src/bin/jp3d/convert.c#192
> > - Several incorrect uses of strcat() before accounting for the
> > lengths:
> > e.g.
> http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/event.c#118
> >
> http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/event.c#132
> > - An incorrect use of sprintf() which can overflow a stack-based
> > buffer:
> http://code.google.com/p/openjpeg/source/browse/trunk/src/lib/openjp3d/event.c#158
> >
> These are stack-based buffer overflows.
> Please use CVE-2013-4290 for these issues.
>
> > Because I found this last one very interesting, I'll paste it here:
> >
> > case 'f' : /* floats */
> > {
> > char tmp[16];
> > double value = va_arg(arg, double);
> > sprintf(tmp, "%f", value);
> > strcat(message, tmp);
> > j += strlen(tmp);
> > ++i;
> > break;
> > }
> >
> > Guess how much space "%f" can take up in an sprintf(3): 317 chars(!)
> >
> > #include <stdio.h>
> > #include <float.h>
> >
> > int main(void) {
> > printf("%f", -DBL_MAX);
> > }
> >
> > $ ./float_length | wc -c
> > 317
> >
> > Some problems are from command-line paramters. Some problems are from
> > image-supplied parameters. I believe even the command-line based
> > problems
> > need CVEs because these tools may be used as part of a web-based image
> > processing mechanism with untrusted users potentially supplying
> > filenames.
> >
> > These links are far from exhaustive -- they were just the ones I
> > happened
> > to easily re-find in the newer codebase. I hope the maintainers have
> > the
> > time and ability to inspect the entire codebase when preparing
> > patches.
> >
> > I believe this needs at least three, maybe four, CVEs:
> >
> > - Integer overflows when allocating memory
> > - Stack-based buffer overflows
> > - Heap-based buffer overflows
> > - Failing to NUL terminate strings copied with strncpy()
> >
> > Thanks,
> > Seth
> >
> >
> > [1] linux-distros () vs openwall org is a private mailing list for
> > distributors of operating systems to collaborate on security
> > vulnerabilities and coordinate security updates. Discussions on
> > linux-distros () vs openwall org are considered private and should not be
> > made public, though the result of the discussions may be made public
> > after the coordinated release date.
> >
> > [2] Please do not release a fix, make public revision control commits,
> > comment in public bug reports or otherwise disclose information about
> > this issue until the coordinated release date. This gives all affected
> > parties a chance to release a fix at the same time.
>
>
> --
> Regards,
>
> Huzaifa Sidhpurwala / Red Hat Security Response
>
>
> --- End Message ---

Attachment: signature.asc
Description: Digital signature