fix(gamefont): Ceil font glyph buffer size to the actual glyph size to prevent a buffer write overflow

[tintinhamans]

The glyph buffer was a fixed uint16[32768]. Store_GDI_Char writes
width * height pixels with no bound check, so a big glyph writes past the end.

A pointSize > 100 cap used to hide this, but it was removed in #3051 so 4K UI scaling
can use bigger fonts. A scaled font can now hit the overflow.

• FontCharsBuffer allocates its pixels and grows to fit the glyph.
• getFont clamps requests to 512 instead of rejecting them, so an oversized
  request still returns a usable font rather than nullptr (callers like
  W3DDisplayString::setFont ignore null and would show no text).

A font size of about 460 is the most any real screen needs (a 48pt heading blown up 9.6x on an 8K display) so I think 512 is a safe value for now.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
🔒 Security Review	✅ Completed 2026-09-07T21:42:55.605071Z	dbebfc4	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Prevent large glyphs from overflowing font buffers

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Prevents large GDI glyphs from overflowing fixed pixel buffers.
• Allocates each backing buffer to fit its triggering glyph.
• Rejects font requests outside the supported 1–512 point range.

Diagram

graph TD
    A["Font request"] --> B{"Size valid?"} -->|No| C["Reject request"]
    B -->|Yes| D["GDI glyph"] --> E{"Buffer fits?"} -->|Yes| F["Store pixels"]
    E -->|No| G["Allocate buffer"] --> F

Loading

High-Level Assessment

The proposed defense-in-depth approach is appropriate: validate externally supplied font sizes and independently size internal storage from measured glyph dimensions. Retaining pooled default buffers for ordinary glyphs avoids unnecessary allocations, while oversized glyphs receive dedicated capacity using the engine's existing allocation conventions.

Files changed (3) +28 / -7

Bug fix (3) +28 / -7

GameFont.cppCap accepted font sizes at 512 points +2/-2

Cap accepted font sizes at 512 points

• Extends font request validation to reject sizes above 512 points while continuing to reject zero and negative sizes. This limits content-driven memory usage without restoring the previous 100-point restriction that blocked high-DPI scaling.

Core/GameEngine/Source/GameClient/GUI/GameFont.cpp

render2dsentence.cppAllocate glyph buffers according to required capacity +19/-3

Allocate glyph buffers according to required capacity

• Adds allocation and cleanup for dynamically sized pixel arrays. Buffer selection now checks each buffer's actual capacity and allocates at least the glyph's width-by-height pixel count, preventing large glyph writes from exceeding the former fixed allocation.

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

render2dsentence.hTrack dynamic font buffer storage and length +7/-2

Track dynamic font buffer storage and length

• Replaces the fixed 32,768-element pixel array with an owned pointer and explicit capacity. Declares lifecycle methods and retains the original constant as the default allocation size for ordinary glyphs.

Core/Libraries/Source/WWVegas/WW3D2/render2dsentence.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Oversized interface text disappears 🐞 Bug ≡ Correctness

Description

FontLibrary::getFont returns nullptr rather than clamping requests above 512, while
display-string consumers silently ignore a null font. When resolution scaling pushes configured
credits, tooltip, or header fonts over the threshold, new strings retain no font and compute zero
extents, while existing strings can retain a stale font.

Code

Core/GameEngine/Source/GameClient/GUI/GameFont.cpp[R182-184]

+	if (pointSize < 1 || pointSize > 512)
  {
  	return nullptr;

Evidence

Resolution scaling can produce a size above 512, and credits pass that value directly to getFont.
A null result is ignored by W3DDisplayString::setFont; after text changes, computeExtents
explicitly assigns zero dimensions when no font is installed, demonstrating the missing-text
outcome.

Core/GameEngine/Source/GameClient/GlobalLanguage.cpp[278-286]
Core/GameEngine/Source/GameClient/Credits.cpp[243-247]
Core/GameEngine/Source/GameClient/GUI/HeaderTemplate.cpp[217-227]
Core/GameEngine/Source/GameClient/Input/Mouse.cpp[621-635]
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp[292-314]
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp[356-375]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Font requests above 512 are rejected with `nullptr`, but callers generally expect a usable font and may consequently display no text. Preserve the allocation limit by clamping positive oversized requests to 512 while continuing to reject invalid non-positive sizes.
## Issue Context
DPI-adjusted font sizes flow directly into `getFont`, and display-string font assignment ignores null values. Clamping enforces the intended maximum glyph allocation without requiring every font consumer to implement fallback behavior.
## Fix Focus Areas
- Core/GameEngine/Source/GameClient/GUI/GameFont.cpp[179-185]
- Core/GameEngine/Source/GameClient/GlobalLanguage.cpp[278-286]
- Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp[292-314]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[greptile-apps]

Greptile Summary

This PR replaces the fixed-size glyph pixel storage with dynamically allocated buffers sized to accommodate each glyph and caps font requests at 512 points.

• Tracks each glyph buffer’s allocated length and uses it when deciding whether another buffer is required.
• Releases dynamically allocated pixel arrays with matching array deletion.
• Clamps oversized requests at the font-library boundary so callers still receive a usable font.

Confidence Score: 5/5

The PR appears safe to merge; the dynamic allocation and matching cleanup address the fixed-buffer overflow without leaving a new actionable failure.

The font library consistently enforces the 512-point limit, while glyph storage allocates at least width × height pixels and checks actual buffer capacity before writing. Pixel arrays are released exactly once through the owning FontCharsClass lifecycle.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameClient/GUI/GameFont.cpp	Clamps positive font requests to 512 points at the common font-creation boundary.
Core/Libraries/Source/WWVegas/WW3D2/render2dsentence.cpp	Allocates glyph buffers according to required capacity, checks their recorded lengths, and releases their pixel arrays correctly.
Core/Libraries/Source/WWVegas/WW3D2/render2dsentence.h	Replaces the fixed pixel array with a value descriptor containing allocated length and pixel pointer.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Request[Font request] --> Clamp[Clamp point size to 512]
    Clamp --> Glyph[Rasterize glyph]
    Glyph --> Size[Compute width × height]
    Size --> Capacity{Current buffer has capacity?}
    Capacity -->|Yes| Store[Store glyph pixels]
    Capacity -->|No| Allocate[Allocate max of default capacity and glyph size]
    Allocate --> Store
    Store --> Destroy[Font destruction]
    Destroy --> Free[Delete each pixel array]

Loading

_{Reviews (6): Last reviewed commit: "refactor(gamefont): Store glyph buffer d..." | Re-trigger Greptile}

[stephanmeesters]

Can't we use std::vector<uint16> Buffer?

At what font size did it overflow the original buffer? Was this leading to crashes?

[tintinhamans]

At what font size did it overflow the original buffer? Was this leading to crashes?

Theoretically somewhere around 130 to 150 point, can also be triggered by something like a custom map with DISPLAY_CINEMATIC_TEXT so might have some security implications.

Can't we use std::vector Buffer?

Each glyph caches a raw pointer into its slab (char_data->Buffer = slab->Buffer + CurrPixelOffset), idk how I'd handle that in a vector.

[xezon]

Needs fixing the slop.

[OmarAglan]

Do this effect PR #3231 ?

[xezon]

Looks safe.