Skip to content

Excessive memory allocation #1111

Open
Open

Description

@dlemstra

The @ImageMagick project is using https://github.com/google/oss-fuzz to find bugs in our own library and in libraries that we use. Attached is a test file that will result in an excessive memory allocation due to this line:

l_nb_tiles = l_cp->tw * l_cp->th;

Adn this makes l_nb_tiles = 1337 * 48 = 64176. We don't know enough about the format but we suspect that a check could be added to do an early exit instead of allocation below

    l_cp->tcps = (opj_tcp_t*) opj_calloc(l_nb_tiles, sizeof(opj_tcp_t));

and the allocations inside the loop

    l_current_tile_param = l_cp->tcps;
    for (i = 0; i < l_nb_tiles; ++i) {

Test file: magick-5304QxxxLsVTCFtl.zip

Activity

  1. rouault commented on Apr 7, 2018

    @rouault
    Collaborator

    The file cannot be read with the master version due to stricter checks.

    ===========================================
    The extension of this file is incorrect.
    FOUND CFtl. SHOULD BE .j2k or .jpc or .j2c
    ===========================================
    
    [INFO] Start to read j2k main header (0).
    [WARNING] Unknown marker
    [WARNING] Cannot take in charge mct data within multiple MCT records
    [WARNING] Unknown marker
    [INFO] Stream reached its end !
    [ERROR] Stream too short
    ERROR -> opj_dump: failed to read the header
    

    That said the underlying problem still exist. OpenJPEG does validate that the number of tiles does not exceed the maximum allowed of 65535 (

    if (l_cp->tw == 0 || l_cp->th == 0 || l_cp->tw > 65535 / l_cp->th) {
    ) , but that still involve significant memory allocations. Not sure there is an easy fix for that

  2. dlemstra commented on Apr 7, 2018

    @dlemstra
    Author

    Sorry for not mentioning but we are using a clone of the master on a daily basis.

    I was wondering if it would be possible to know the minimum size of a tile and check if the input data is long enough.

  3. rouault commented on Apr 7, 2018

    @rouault
    Collaborator

    I was wondering if it would be possible to know the minimum size of a tile and check if the input data is long enough.

    I've managed to create a file of 65535x1 pixels with tiles of size 1x1, so 65535 tiles. Its size is just 3.6 MB, so this is not an effective way of avoiding the issue.

  4. dlemstra commented on Apr 8, 2018

    @dlemstra
    Author

    Our file is only 244 bytes and I was wondering the allocation could be moved to opj_jp2_read_header_procedure and do the allocation right before the return OPJ_TRUE;. And before the allocation you could even check if the stream is at least 64176 * number of bytes required for single tile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions