Skip to content

j2k_image_comptparm's sgnd member has a different type from opj_image_comptparm in <openjpeg.h> #632

Description

@RobFerrio

In TileWriter.h from the j2k module, the following j2k_image_comptparm struct should mimic the opj_image_comptparm struct in openjpeg.h:

typedef struct _j2k_image_comptparm // c.f. opj_image_comptparm in <openjpeg.h>
{
    uint32_t dx;     /** XRsiz: horizontal separation of a sample of ith component with respect to the reference grid */
    uint32_t dy;     /** YRsiz: vertical separation of a sample of ith component with respect to the reference grid */
    uint32_t w;     /** data width */
    uint32_t h;     /** data height */
    uint32_t x0;     /** x component offset compared to the whole image */
    uint32_t y0;     /** y component offset compared to the whole image */
    uint32_t prec;     /** precision */
    uint32_t bpp;  /** image depth in bits */
    J2K_BOOL sgnd;     /** signed (1) / unsigned (0) */     <==== Type is J2K_BOOL = NRT_BOOL = bool in c++
} j2k_image_comptparm;

but the openjpeg struct has sgnd member of type OPJ_UINT32 (uint32_t):

/**
 * Component parameters structure used by the opj_image_create function
 * */
typedef struct opj_image_comptparm {
    /** XRsiz: horizontal separation of a sample of ith component with respect to the reference grid */
    OPJ_UINT32 dx;
    /** YRsiz: vertical separation of a sample of ith component with respect to the reference grid */
    OPJ_UINT32 dy;
    /** data width */
    OPJ_UINT32 w;
    /** data height */
    OPJ_UINT32 h;
    /** x component offset compared to the whole image */
    OPJ_UINT32 x0;
    /** y component offset compared to the whole image */
    OPJ_UINT32 y0;
    /** precision */
    OPJ_UINT32 prec;
    /** image depth in bits */
    OPJ_UINT32 bpp;
    /** signed (1) / unsigned (0) */
    OPJ_UINT32 sgnd;     <==== Type is OPJ_UINT32 = uint32_t
} opj_image_cmptparm_t;

This mismatch can cause a type-punning bug inside OpenJPEGImpl.c in j2k_image_tile_create where only 1byte of the 4 of sgnd is set to 0, while the content of the padding bytes is non-deterministic/compiler dependent, and can be influenced by other threads during the compression process.
This can lead to random pixel shifts of +128 since the garbage written inside sgnd makes it evaluate to signed even if the original pixel values were unsigned.

This problem can be consistently replicated by poisoning freed memory (for example with MALLOC_PERTURB_ in glibc).

The bug can be easily resolved by changing the sgnd type in TileWriter.h:

-    J2K_BOOL sgnd;
+    uint32_t sgnd;

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions