Skip to content

fix(htree): Reject invalid W3D hierarchy pivot counts - #3195

Merged
xezon merged 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/htree-pivot-count-overflow
Sep 1, 2026
Merged

fix(htree): Reject invalid W3D hierarchy pivot counts#3195
xezon merged 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/htree-pivot-count-overflow

Conversation

@CryoTheRenegade

@CryoTheRenegade CryoTheRenegade commented Aug 24, 2026

Copy link
Copy Markdown

HTreeClass::Load_W3D expects every loaded hierarchy to contain at least one pivot because later code accesses Pivot[0].

Previously, a count of zero skipped allocation but allowed loading to continue. Malformed pre-3.0 files could also reach that state by setting NumPivots to UINT32_MAX, which wraps to zero when the loader adds the synthetic root pivot.

This change validates the adjusted pivot count before allocation. Counts below one now return LOAD_ERROR, while valid hierarchies follow the existing allocation and loading path.

This prevents corrupt or crafted W3D assets from leaving an HTreeClass with no pivot storage and crashing when Pivot[0] is used.

Found by clang-tidy.

Prevent the synthetic-root increment from wrapping to zero so load does not skip allocation and then write Pivot[0].

Co-authored-by: Cursor <cursoragent@cursor.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Reject overflowing pre-3.0 W3D hierarchy pivot counts

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Rejects malformed pre-3.0 hierarchy counts before synthetic-root arithmetic can overflow.
• Guards synthetic-root initialization against missing or empty pivot allocations.
Diagram

graph TD
  A["W3D Header"] --> B{"Pre-3.0?"} -->|Yes| C{"Count max?"} -->|No| E["Allocate Pivots"] --> F["Read Pivots"] --> G{"Synthetic root?"} -->|Yes, valid| H["Initialize Root"]
  B -->|No| E
  C -->|Yes| D["Load Error"]
Loading
High-Level Assessment

The current defense-in-depth approach is appropriate: reject the invalid count before arithmetic, then enforce the allocation invariant at the write site. Relying on only the downstream guard would avoid the null write but would not reject the malformed header at its source.

Files changed (1) +7 / -0

Bug fix (1) +7 / -0
htree.cppPrevent legacy pivot-count overflow and null writes +7/-0

Prevent legacy pivot-count overflow and null writes

• Rejects UINT32_MAX pivot counts before pre-3.0 synthetic-root adjustment can wrap to zero. Verifies pivot storage exists and contains at least one element before writing the synthetic RootTransform pivot.

Core/Libraries/Source/WWVegas/WW3D2/htree.cpp

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread Core/Libraries/Source/WWVegas/WW3D2/htree.cpp Outdated
Comment thread Core/Libraries/Source/WWVegas/WW3D2/htree.cpp Outdated
Comment thread Core/Libraries/Source/WWVegas/WW3D2/htree.cpp Outdated
Comment thread Core/Libraries/Source/WWVegas/WW3D2/htree.cpp Outdated
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown

Greptile Summary

The PR hardens W3D hierarchy loading against a pre-3.0 pivot-count overflow by rejecting the resulting zero count before allocation.

  • Requires every loaded hierarchy to contain at least one pivot after legacy root adjustment.
  • Allocates the pivot array only after validating the adjusted count.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up scope.

No blocking failure remains.

Important Files Changed

Filename Overview
Core/Libraries/Source/WWVegas/WW3D2/htree.cpp Validates the adjusted hierarchy pivot count before allocating and reading pivots, preventing the malformed pre-3.0 null-write path.

Reviews (2): Last reviewed commit: "style(htree): Remove redundant validatio..." | Re-trigger Greptile

Comment thread Core/Libraries/Source/WWVegas/WW3D2/htree.cpp Outdated
@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Fix Is fixing something, but is not user facing labels Sep 1, 2026
@xezon

xezon commented Sep 1, 2026

Copy link
Copy Markdown

The change title and description are outdated.

@CryoTheRenegade CryoTheRenegade changed the title fix(htree): Reject UINT32_MAX pivot counts on pre-3.0 W3D loads fix(htree): Reject invalid W3D hierarchy pivot counts Sep 1, 2026
@CryoTheRenegade

Copy link
Copy Markdown
Author

updated

@xezon
xezon merged commit 3d7b15f into TheSuperHackers:main Sep 1, 2026
23 checks passed
@CryoTheRenegade
CryoTheRenegade deleted the fix/htree-pivot-count-overflow branch September 1, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants