Skip to content

Bound the SIZE chunk before its dimensions reach the allocation (fixes #87) - #88

Open
jcelerier wants to merge 1 commit into
jpaver:masterfrom
jcelerier:fix/vox-size-overflow
Open

jcelerier wants to merge 1 commit into
jpaver:masterfrom
jcelerier:fix/vox-size-overflow

Conversation

@jcelerier

@jcelerier jcelerier commented Aug 30, 2026 •

Copy link
Copy Markdown

Fixes #87.

ogt_vox_read_scene() validates size_x/size_y/size_z only for non-zero, so a crafted SIZE chunk reaches:

uint32_t voxel_count = size_x * size_y * size_z;
_vox_calloc(sizeof(ogt_vox_model) + voxel_count);

with dimensions whose product wraps. 65536 x 65536 x 1 overflows uint32 to 0, the allocation collapses to sizeof(ogt_vox_model), and the XYZI loop — whose only bound is x < size_x && y < size_y && z < size_z, all of which hold — writes the voxel payload far past it.

AddressSanitizer:

ERROR: AddressSanitizer: heap-buffer-overflow
WRITE of size 1, 0 bytes after a 24-byte region
    ogt_vox.h:1590 in ogt_vox_read_scene_with_flags

Under NDEBUG the surrounding ogt_asserts are compiled out, so this is reachable in release builds from a file alone.

Why each axis is bounded before the product

Widening the multiply is not sufficient on its own. Three uint32 dimensions need up to 96 bits, so 0x40000000 cubed is 2^90 — which is 2^26 once truncated to uint64, and passes any check made on the product. The bound has to be applied per axis, before the multiplication.

The limits (65536 per axis, 2^30 voxels) are far above what MagicaVoxel itself can author — 256 per axis — and exist only so malformed input cannot reach the arithmetic.

Verification

Found via a .vox robustness suite in ossia/score that derives malformed files from valid ones and loads each in a forked child. With this patch the crafted-SIZE case is rejected cleanly instead of corrupting the heap, and the suite is green at 3407 assertions.

Separately, and not part of this PR: consumers who override ogt_assert to be non-fatal — as the header documents — need to handle a scene coming back with an instance whose model_index is out of range, or whose model is NULL. That is caller-side; we handle it on our end.

@jcelerier
jcelerier force-pushed the fix/vox-size-overflow branch from 80d04dc to d0a6630 Compare August 30, 2026 18:45
ogt_vox_read_scene() validates size_x/size_y/size_z only for non-zero, so a
crafted SIZE chunk reaches

    uint32_t voxel_count = size_x * size_y * size_z;
    _vox_calloc(sizeof(ogt_vox_model) + voxel_count);

with dimensions whose product wraps. 65536 x 65536 x 1 overflows uint32 to 0,
the allocation collapses to sizeof(ogt_vox_model), and the XYZI loop -- whose
only bound is x < size_x && y < size_y && z < size_z, all of which hold --
then writes the voxel payload far past it. AddressSanitizer reports a
heap-buffer-overflow WRITE of size 1 just past the 24-byte region. Under
NDEBUG the surrounding ogt_asserts are compiled out, so this is reachable in
release builds from a file alone.

Bound each axis before the product is formed rather than after. Widening the
multiply alone is not sufficient: three uint32 dimensions need up to 96 bits,
so 0x40000000 cubed is 2^90, which is 2^26 in uint64 and would pass any check
made on the product.

The limits are far above what MagicaVoxel itself can author -- 256 per axis --
and exist only so that malformed input cannot reach the arithmetic.

Found with a .vox robustness suite that derives malformed files from valid
ones and loads each in a forked child.

Fixes jpaver#87.
@jcelerier
jcelerier force-pushed the fix/vox-size-overflow branch from d0a6630 to b95bc54 Compare August 30, 2026 19:12
jcelerier added a commit to ossia/score that referenced this pull request Aug 31, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 1, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 1, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 1, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 2, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 4, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 4, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 8, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 8, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 8, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 8, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit to ossia/score that referenced this pull request Sep 8, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
pull Bot pushed a commit to Spencerx/score that referenced this pull request Sep 9, 2026
Both upstream PRs were rewritten to a single commit with the rationale moved out
of the source and into the commit message, so the SHAs this tree pinned no
longer exist:

  opengametools  80d04dc -> d0a6630   (jpaver/opengametools#88)
  vcglib         a200b72 -> 346dfc8   (cnr-isti-vclab/vcglib#255)

Verified that only comments changed: diffing old against new and filtering out
comment lines leaves one whitespace realignment of k_vox_max_dimension in
opengametools and nothing at all in vcglib.

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

ogt_vox: SIZE chunk missing upper-bound validation allows integer overflow → heap OOB write via crafted .vox file (ogt_vox.h:1590)

1 participant