This issue was caused by the updated version of OpenJPEG. Root cause analysis:

--------------------------------------
(1) opj_alloc_tile_component_data
--------------------------------------
``opj_malloc`` was replaced with ``opj_image_data_alloc`` in function ``opj_alloc_tile_component_data``.

OPJ_BOOL opj_alloc_tile_component_data(opj_tcd_tilecomp_t *l_tilec)
{
    if ((l_tilec->data == 00) ||
            ((l_tilec->data_size_needed > l_tilec->data_size) &&
             (l_tilec->ownsData == OPJ_FALSE))) {
        l_tilec->data = (OPJ_INT32 *) opj_image_data_alloc(l_tilec->data_size_needed);      // --> diff
        if (! l_tilec->data) {
            return OPJ_FALSE;
        }
        /*fprintf(stderr, "tAllocate data of tilec (int): %d x OPJ_UINT32n",l_data_size);*/
        l_tilec->data_size = l_tilec->data_size_needed;
        l_tilec->ownsData = OPJ_TRUE;
    } else if (l_tilec->data_size_needed > l_tilec->data_size) {
        // skipped...
    }
    return OPJ_TRUE;
}

--------------------------------------
(2) opj_image_data_alloc
--------------------------------------
``opj_image_data_alloc`` was implemented by ``opj_aligned_malloc``.

void* OPJ_CALLCONV opj_image_data_alloc(OPJ_SIZE_T size)
{
    void* ret = opj_aligned_malloc(size);
    /* printf("opj_image_data_alloc %p\n", ret); */
    return ret;
}

--------------------------------------
(3) opj_aligned_malloc
--------------------------------------
The actual implementation of function ``opj_aligned_malloc`` depends on environments. The details can be found in file ``opj_malloc.h``.


--------------------------------------
(4) opj_free
--------------------------------------
However, ``opj_free`` never points to ``opj_aligned_free``. Thus, if ``opj_image_data_alloc`` doesn't point to ``opj_malloc`` but ``opj_free`` points to ``free``, the mismatch could crash the process.

void sycc422_to_rgb(opj_image_t* img) {
  // skipped...
  opj_free(img->comps[0].data);
  opj_free(img->comps[1].data);
  opj_free(img->comps[2].data);
  // skipped...
}

``opj_image_data_alloc`` should be paired with ``opj_image_data_free``.
(extracted from openjpeg.h)

/**
 * Allocator for opj_image_t->comps[].data
 * To be paired with opj_image_data_free.
 *
 * @param   size    number of bytes to allocate
 *
 * @return  a new pointer if successful, NULL otherwise.
 * @since 2.2.0
*/
OPJ_API void* OPJ_CALLCONV opj_image_data_alloc(OPJ_SIZE_T size);

/**
 * Destructor for opj_image_t->comps[].data
 * To be paired with opj_image_data_alloc.
 *
 * @param   ptr    Pointer to free
 *
 * @since 2.2.0
*/
OPJ_API void OPJ_CALLCONV opj_image_data_free(void* ptr);

Here the issue exists in fx_codec_jpx_opj.cpp.
The same issue can be found in OpenJPEG's image.c and jp2.c.

To keep the compatibility with OpenJPEG's new API, we should use ``opj_image_data_free`` for all ``opj_image_t->comps[].data``.

I uploaded a patch to address this issue.

Deleted: 762374.patch 5.4 KB

762374.patch 5.4 KB Download

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=4953980643049472.

(There probably needs to be an upstream bug as well).

#5 ClusterFuzz may not be able to reproduce this issue since the behavior of ``opj_aligned_malloc`` depends on concrete environment. I can reproduce it on Windows.

#7 I'll report it to upstream.

How would you like to be credited in the git commit log entry?

I can't seem to reproduce the bug on Chrome 60 on Windows, or with pdfium_test or Chrome, tip-of-tree, on Linux, with or without ASan.

How are you reproducing it?

Hi palmer, the issue was introduced by commit https://pdfium.googlesource.com/pdfium/+/088ca03f25fe1f6d75c0ff3b71e0ad3d018a5e0c

My environments:
OS: Windows 7 x64
Compiler: VS 2015
pdfium_test: x86 version (Release)
PageHeap: enabled within gflags.exe

----------------------
args.gn for ninja
----------------------
use_goma = false
pdf_use_skia = false
pdf_enable_xfa = false
pdf_enable_v8 = false
pdf_is_standalone = true
is_debug = false
target_cpu = "x86"
treat_warnings_as_errors = false

I can confirm that page heap must be enabled to reproduce this issue.

#9 You can credit to Ke Liu of Tencent's Xuanwu Lab if my patch works fine. Thank you.

There's a similar issue that has been fixed in OpenJPEG, the commit record is https://github.com/uclouvain/openjpeg/commit/61fb5dd7f81c2e3dfabbb99f59dc89572d59fa37

Also, I reported this issue to upstream at https://github.com/uclouvain/openjpeg/issues/1014

Pull request: https://github.com/uclouvain/openjpeg/pull/1016

Hi, my patch has been adopted by upstream. You can find it at https://github.com/uclouvain/openjpeg/commit/b73ce715d2a484d7355639d863d0418a0e5b8858

I've got the patch up for review now at https://pdfium-review.googlesource.com/13610

Isn't this M-63 since M-62 is branch 3202 and it branched at r499098? Whereas the OpenJPEG update happened after PDFium branch origin/chromium/3204, and didn't get rolled into Chromium until r499763.

Note: https://github.com/uclouvain/openjpeg/commit/b73ce715d2a484d7355639d863d0418a0e5b8858 only patches 1 location in jp2.c, while https://pdfium-review.googlesource.com/13610 (Ke's patch) patches 2 locations in jp2.c.  Do we know why this discrepancy exists?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55dc1f8189d76c7ed7d9d3878c22c08e300669e3

commit 55dc1f8189d76c7ed7d9d3878c22c08e300669e3
Author: pdfium-deps-roller@chromium.org <pdfium-deps-roller@chromium.org>
Date: Mon Sep 11 21:02:51 2017

Roll src/third_party/pdfium/ 131c0eb2e..56ec0818c (1 commit)

https://pdfium.googlesource.com/pdfium.git/+log/131c0eb2e34e..56ec0818c3ed

$ git log 131c0eb2e..56ec0818c --date=short --no-merges --format='%ad %ae %s'
2017-09-11 palmer Use the right allocate and free functions in OpenJPEG.

Created with:
  roll-dep src/third_party/pdfium
BUG= 762374 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


TBR=dsinclair@chromium.org

Change-Id: Id0d43903dbc84611c3aaae9e28ca65896cf40de0
Reviewed-on: https://chromium-review.googlesource.com/660679
Reviewed-by: <pdfium-deps-roller@chromium.org>
Commit-Queue: <pdfium-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501040}
[modify] https://crrev.com/55dc1f8189d76c7ed7d9d3878c22c08e300669e3/DEPS

#21 Hi palmer, the discrepancy was introduced by https://pdfium.googlesource.com/pdfium/+/c37d7d452d6a37c997c8709576dd71406ecff618

In 0022-jp2_apply_pclr_overflow.patch, we use ``if`` statements to check whether ``src`` or ``dst`` point to null, the previously allocated data will be freed immediately once null pointer detected. Thus, we introduced ``opj_free(new_comps[j].data)`` here.

In OpenJPEG, ``assert`` was used instead of ``if``, which could lead to null pointer access in Release version.

That's why we patched 2 locations in jp2.c but OpenJPEG only patched 1.

*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

Nice one stackexploit@! The VRP panel awarded $5,000 and a $1,337 bonus for this bug.

This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

+awhalley@ (Security TPM) for M63 merge review

No M63 merge needed.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot