Skip to content

Fix truncated size and stale capacity in tile array allocation - #258

Merged
kpchoi merged 1 commit into
AcademySoftwareFoundation:mainfrom
alejandro-arango-epicgames:fix_tile_alloc
Aug 7, 2026
Merged

Fix truncated size and stale capacity in tile array allocation#258
kpchoi merged 1 commit into
AcademySoftwareFoundation:mainfrom
alejandro-arango-epicgames:fix_tile_alloc

Conversation

@jsperrier1

Copy link
Copy Markdown
Contributor

Two defects in the tile array allocation added in #236, one of them a memory-safety issue reachable from the bitstream. Both occur at the encoder site in enc_frm_prepare() and the decoder site in dec_frm_prepare().

1. Truncated allocation size

The array is sized with a size_t expression, but oapv_ops_mem.malloc takes a 32-bit size:

void *(*malloc)(void *udata, unsigned int size);   // inc/oapv.h

so the value is silently truncated on the way in:

ctx->tile = (oapvd_tile_t *)oapv_ops_malloc(ctx, sizeof(oapvd_tile_t) * ctx->num_tiles);
oapv_assert_rv(ctx->tile != NULL, OAPV_ERR_OUT_OF_MEMORY);
oapv_mset(ctx->tile, 0, sizeof(oapvd_tile_t) * ctx->num_tiles);   // full, untruncated size

The oapv_mset() on the following line uses the untruncated size, so a request above 4 GiB under-allocates and is then immediately written past its end.

Reachability of the decoder path. sizeof(oapvd_tile_t) is 88, so the product exceeds 2^32 once num_tiles reaches 48,806,447. Under an UNCONST profile at the minimum tile size that needs a frame of roughly 16.7M x 745 pixels — both inside the u(24) frame dimensions, and accepted by oapv_validate_tile_topology() because the count stays below INT_MAX. Since tile_size_present_in_fh_flag may be 0, no per-tile payload is required: a frame header of a few dozen bytes is enough.

2. Stale capacity after a failed allocation

Both sites freed the old array, then returned early on allocation failure with tile_cap still holding the previous capacity:

oapv_ops_free(ctx, ctx->tile);
ctx->tile = oapv_ops_malloc(...);                              // fails -> NULL
oapv_assert_rv(ctx->tile != NULL, OAPV_ERR_OUT_OF_MEMORY);     // returns
ctx->tile_cap = ctx->num_tiles;                                // never reached

A later call whose tile count fits that stale capacity skips the whole block and dereferences ctx->tile while NULL, turning a recoverable OAPV_ERR_OUT_OF_MEMORY into a crash.

The change

Range-check the byte size before allocating, and clear the pointer and the capacity together so they cannot disagree. The decoder reports OAPV_ERR_MALFORMED_BITSTREAM (the count comes from the bitstream); the encoder reports OAPV_ERR_INVALID_ARGUMENT (it comes from the caller's parameters).

One file, +25/-4. ctest 18/18 pass.

The tile array is sized with a size_t expression but the instance
allocator takes a 32-bit size, so the value is silently truncated on the
way in. In the decoder the oapv_mset() on the following line uses the
full, untruncated size, so a request above 4 GiB under-allocates and is
then written past its end.

The decoder path is reachable from the bitstream. sizeof(oapvd_tile_t)
is 88, so the product exceeds 2^32 once num_tiles reaches 48,806,447,
which an UNCONST profile at the minimum tile size reaches with a frame
of about 16.7M x 745 pixels -- both within the u(24) frame dimensions,
and accepted by oapv_validate_tile_topology() since the count stays
below INT_MAX. tile_size_present_in_fh_flag may be 0, so no per-tile
payload is needed either: a frame header of a few dozen bytes is enough
to trigger it.

Both sites also left tile_cap holding the previous capacity when the
allocation failed, after the old array had already been freed. A later
call whose tile count fitted that stale capacity skipped the allocation
and dereferenced the NULL pointer, turning a recoverable
OAPV_ERR_OUT_OF_MEMORY into a crash.

Range-check the byte size before allocating, and clear the pointer and
the capacity together so they cannot disagree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cpncf cpncf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kpchoi kpchoi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kpchoi
kpchoi merged commit 4760f49 into AcademySoftwareFoundation:main Aug 7, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants