Skip to content

bugfix(worldheightmap): Reject map sizes that overflow the cell array allocations - #3486

Merged
xezon merged 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/fix/map-dimension-bounds
Oct 15, 2026
Merged

xezon merged 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/fix/map-dimension-bounds

Conversation

@tintinhamans

@tintinhamans tintinhamans commented Oct 12, 2026 •

Copy link
Copy Markdown

ParseHeightMapData takes width and height straight from the map file and only checks the data size against m_width*m_height. That product wraps in 32 bits, so absurd dimensions pass and PathfindLayer, PartitionManager and W3DShroud allocate the wrapped count while indexing with the real one.

Both height map parsers now reject an area above 4096x4096, a side above 8192 or a border that does not fit, before allocating. The side limit keeps the shroud texture and the 16 bit shoreline coordinates in range. The largest of the 446 retail and custom maps I tested is 800.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3a4d3245-46b8-42e0-a960-7477bca1b372
📥 Commits

Reviewing files that changed from the base of the PR and between 1456efb and 96bdbe0.

📒 Files selected for processing: 1
  • Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

Walkthrough

Height map parsing now validates dimensions, border sizes, and declared data sizes in both full-data and size-only paths. Invalid values produce ERROR_CORRUPT_FILE_FORMAT. Each path validates the data size before allocating the height buffer.

Changes

Height map validation

Layer / File(s) Summary
Validate height map parsing
Core/GameEngine/Include/Common/MapReaderWriterInfo.h, Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp
The parsers enforce dimensions from 1 through MAX_MAP_DIMENSION (8192), limit map area to MAX_MAP_AREA, and validate border sizes. Each parser rejects nonpositive data sizes, sizes unequal to width * height, and sizes greater than the remaining chunk bytes before allocating the buffer.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: mauller

Merge Risk: ⚪ Minimal · up to 96bdb

The height-map parsers now reject the identified oversized and truncated inputs before allocating their height buffers. No actionable merge-blocking risk remains from the supplied evidence.

Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Description check Passed The description clearly explains the overflow risk and the new validation for map dimensions, area, border size, and allocation safety.
Title check Passed The title clearly identifies the main change: rejecting world-height-map sizes that can overflow cell-array allocations.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp Outdated
@tintinhamans
tintinhamans force-pushed the arctic/fix/map-dimension-bounds branch from c383ea9 to a5bae28 Compare October 12, 2026 02:27
@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Critical impact] The PR appears safe to merge.

Summary

The PR rejects oversized maps and invalid borders before allocating height data.

  • Height-map readers reject oversized maps before allocating height data.

Reviews (7) · Last reviewed commit: "bugfix(worldheightmap): Reject map sizes..." · Reviewed by Greptile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7d66925a-87e4-4a90-b43f-92d527b561ae
📥 Commits

Reviewing files that changed from the base of the PR and between 0e6a06c and a5bae28.

📒 Files selected for processing (1)
  • Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

@tintinhamans
tintinhamans force-pushed the arctic/fix/map-dimension-bounds branch from a5bae28 to a8ee0e9 Compare October 12, 2026 02:43
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp Outdated
@tintinhamans
tintinhamans force-pushed the arctic/fix/map-dimension-bounds branch from a8ee0e9 to 6d6fdf8 Compare October 13, 2026 07:34
@Mauller

Mauller commented Oct 13, 2026

Copy link
Copy Markdown

Although world builder breaks at the moment when going beyond 1200x1200 due to memory limits, you might want to make sure world builder also adheres to the maximum world dimensions.

@tintinhamans

Copy link
Copy Markdown
Author

Although world builder breaks at the moment when going beyond 1200x1200 due to memory limits, you might want to make sure world builder also adheres to the maximum world dimensions.

I'll cap WorldBuilder at the same limit in a follow-up and look at the memory issue.

@tintinhamans tintinhamans added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Memory Is memory related labels Oct 13, 2026
@tintinhamans
tintinhamans force-pushed the arctic/fix/map-dimension-bounds branch from 6d6fdf8 to 8974823 Compare October 13, 2026 13:14

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3665e2f8-4cc3-461f-a1fd-0ddb7ef65fd3
📥 Commits

Reviewing files that changed from the base of the PR and between 6d6fdf8 and 8974823.

📒 Files selected for processing (2)
  • Core/GameEngine/Include/Common/MapReaderWriterInfo.h
  • Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Comment thread Core/GameEngine/Include/Common/MapReaderWriterInfo.h Outdated
Comment thread Core/GameEngine/Include/Common/MapReaderWriterInfo.h Outdated
@tintinhamans
tintinhamans force-pushed the arctic/fix/map-dimension-bounds branch from 8974823 to 1456efb Compare October 13, 2026 16:29

@Mauller Mauller left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp Outdated
Comment thread Core/GameEngine/Include/Common/MapReaderWriterInfo.h
@tintinhamans
tintinhamans force-pushed the arctic/fix/map-dimension-bounds branch from 1456efb to 96bdbe0 Compare October 13, 2026 18:49
@tintinhamans
tintinhamans force-pushed the arctic/fix/map-dimension-bounds branch from 96bdbe0 to db81624 Compare October 13, 2026 19:23
@xezon
xezon merged commit 5d69c00 into TheSuperHackers:main Oct 15, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Gen Relates to Generals Memory Is memory related Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants