pgx_dec.c: shift exponent UB in pgx_wordtoint() (reopen of #334 with PoC)

[JorgeBarredo14]

Describe the bug

pgx_wordtoint at src/libjasper/pgx/pgx_dec.c does two unguarded shifts of 1 by prec and prec-1, where prec is attacker-controlled from the PGX header. When prec is 31 or 32 the shift is UB.

Current code on master:

static jas_seqent_t pgx_wordtoint(uint_fast32_t v, int prec, bool sgnd)
{
    jas_seqent_t ret;
    v &= (1 << prec) - 1;
    ret = (sgnd && (v & (1 << (prec - 1)))) ? (v - (1 << prec)) : v;
    return ret;
}

UBSan on a crafted .pgx with prec = 32:

pgx_dec.c:513:10: runtime error: shift exponent 32 is too large for 32-bit type 'int'
pgx_dec.c:514:25: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'

This is the same class of bug as #334, which was closed as "cannot reproduce on master / assume fixed". The code has not been fixed; I have a PoC that reproduces on current master.

To Reproduce

Attached PoC: poc_pgx_shift.pgx (zipped because GitHub blocks .pgx uploads).

Build with:

./configure
make
CFLAGS="-fsanitize=undefined -fno-sanitize-recover=all"

Then run any consumer that calls jas_image_decode on the file. UBSan reports the shift UB immediately.

Expected behavior

Reject precisions outside the supported range, or perform the shift in an unsigned type wide enough to hold 1 << 32.

Fix idea

--- a/src/libjasper/pgx/pgx_dec.c
+++ b/src/libjasper/pgx/pgx_dec.c
@@ -510,9 +510,17 @@
 static jas_seqent_t pgx_wordtoint(uint_fast32_t v, int prec, bool sgnd)
 {
     jas_seqent_t ret;
-    v &= (1 << prec) - 1;
-    ret = (sgnd && (v & (1 << (prec - 1)))) ? (v - (1 << prec)) : v;
+    /* prec comes from the file header; clamp and use wide shifts. */
+    if (prec <= 0 || prec > 32) {
+        return 0;
+    }
+    const uint_fast32_t mask = (prec == 32)
+        ? UINT32_MAX
+        : (((uint_fast32_t)1 << prec) - 1);
+    v &= mask;
+    const uint_fast32_t sign_bit = (uint_fast32_t)1 << (prec - 1);
+    ret = (sgnd && (v & sign_bit))
+        ? (jas_seqent_t)(v - sign_bit - sign_bit)
+        : (jas_seqent_t)v;
     return ret;
 }

The same bound check prec <= 0 || prec > 32 should also be applied wherever prec is first read from the PGX header, so callers can bail out early.

Related: same pattern exists in bitstoint at jas_image.c and I will file a separate issue for it.

Refs: #270, #279, #283, #334.

PoC is zipped because GitHub blocks .pgx uploads. Unzip to get poc_pgx_shift.pgx.

poc_pgx_shift.zip