fix(SEC-2): replace input-sized C VLAs with heap allocation at three sites

[kostub]

Summary

Fixes SEC-2 from issues.md: three C variable-length arrays (VLAs) sized by
attacker/render-controlled input could silently overflow the thread stack —
undefined behavior that manifests as a hard EXC_BAD_ACCESS/SIGSEGV crash
with no recoverable NSException.

Changed sites (2 files):

• iosMath/lib/MTMathAtomFactory.m — +[MTMathAtomFactory mathListForCharacters:]:
  unichar buff[len] → unichar *buff = malloc(sizeof(unichar) * len) + free(buff).
  len is public-API-caller-controlled; a multi-megabyte string put 2×N bytes on the stack.

• iosMath/render/internal/MTTypesetter.m — changeFont():
  unichar charBuffer[str.length] → malloc + free.
  str is atom.nucleus; fused atom runs or a single large-nucleus atom make it input-sized.

• iosMath/render/internal/MTTypesetter.m — -[MTTypesetter makeTable:]:
  CGFloat columnWidths[numColumns] → CGFloat *columnWidths = calloc(numColumns, sizeof(CGFloat)) + free(columnWidths).
  calloc zero-inits, replacing the previous memset. No helper method signature changes.

Behavior: Identical. No API, header, or method signature changes.

Tests

Three new regression tests added to MTTypesetterTest:

• testMathListForCharactersLargeInput_SEC2 — 10k-character digit string returns 10k atoms
• testChangeFontLargeNucleus_SEC2 — 10k-char variable atom renders to a non-nil display
• testMathTableManyColumns_SEC2 — 500-column table typesets without crash

All existing tests (MTFontManagerTest, MTMathAtomTest, MTMathListBuilderTest,
MTMathListRangeTest, MTMathListTest, MTTypesetterTest) continue to pass.

Test plan

• swift build — clean build, no warnings
• xcodebuild test iOS scheme — all tests passed
• Three new SEC-2 regression tests pass
• Every malloc/calloc has a matching free on all return paths

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request replaces Variable Length Arrays (VLAs) with heap allocations (malloc and calloc) in MTMathAtomFactory.m and MTTypesetter.m to prevent stack overflows on large inputs, and adds corresponding regression tests. The reviewer feedback suggests optimizing these changes by implementing a hybrid stack/heap allocation pattern. Since these methods are frequently called with small inputs, unconditionally allocating on the heap introduces performance overhead. Using a small stack buffer for typical sizes and falling back to heap allocation only for exceptionally large inputs avoids this overhead while maintaining safety.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

changeFont is a very hot path called frequently during typesetting (often for single-character nuclei like variables or numbers). Unconditionally allocating memory on the heap via malloc for every single call introduces a significant performance regression.

Implementing a hybrid stack/heap allocation pattern avoids this overhead for typical short strings (length <= 256) while still safely preventing stack overflows for exceptionally large inputs.

    unichar stackBuffer[256];
    unichar *charBuffer = stackBuffer;
    if (length > 256) {
        charBuffer = malloc(sizeof(unichar) * (size_t)length);
        if (!charBuffer) {
            return @"";
        }
    }

[gemini-code-assist]

Free the buffer only if it was allocated on the heap.

    if (charBuffer != stackBuffer) {
        free(charBuffer);
    }

[kostub]

Addressed in db50120. Wrapped the alloc..use region in @try { ... } @finally { free(charBuffer); } so the buffer is released on all exit paths, including the IllegalCharacter / Invalid style exceptions that styleCharacter() can @throw from inside the loop. (Kept unconditional heap alloc rather than the stack/heap hybrid — the blocking concern here was the unwind-path leak.)

[gemini-code-assist]

Unconditional heap allocation via malloc for every call to mathListForCharacters: introduces unnecessary performance overhead, especially for typical short math strings (which are usually well under 100 characters). Additionally, if malloc fails (returning NULL), passing NULL to getCharacters:range: will cause a crash.

Using a hybrid stack/heap allocation pattern (using a small stack buffer for common short lengths and only falling back to malloc for exceptionally large inputs) avoids heap allocation overhead in the vast majority of cases while safely handling large inputs and potential allocation failures.

    unichar stackBuff[256];
    unichar *buff = stackBuff;
    if (len > 256) {
        buff = malloc(sizeof(unichar) * (size_t)len);
        if (!buff) {
            return [[MTMathList alloc] init];
        }
    }

[gemini-code-assist]

Free the buffer only if it was allocated on the heap.

    if (buff != stackBuff) {
        free(buff);
    }

[gemini-code-assist]

Most math tables have a very small number of columns (typically under 10). Unconditionally allocating the columnWidths array on the heap via calloc introduces unnecessary overhead.

Using a hybrid stack/heap allocation pattern avoids heap allocation for typical tables while safely falling back to calloc for tables with a very large number of columns.

    CGFloat stackColumnWidths[32];
    CGFloat *columnWidths = stackColumnWidths;
    if (numColumns > 32) {
        columnWidths = calloc(numColumns, sizeof(CGFloat));
        if (!columnWidths) {
            return [[MTMathListDisplay alloc] initWithDisplays:[NSArray array] range:table.indexRange];
        }
    } else {
        memset(stackColumnWidths, 0, sizeof(stackColumnWidths));
    }

[gemini-code-assist]

Free the buffer only if it was allocated on the heap.

    if (columnWidths != stackColumnWidths) {
        free(columnWidths);
    }

[kostub]

Addressed in db50120. Wrapped the makeTable: body in @try { ... } @finally { free(columnWidths); }. typesetCells:/makeRowWithColumns: reach changeFont, which can @throw, so the normal-return-only free leaked on unwind. The tableDisplay is now hoisted out of the @Try and returned after free.

[kostub]

EM-REVIEW v1

SEC-2: VLA -> heap allocation review

Scope reviewed: the three converted sites and the three regression tests. The core change is sound -- heap allocation removes the unbounded-stack-growth risk from input-controlled sizes, and calloc zero-init correctly replaces the prior memset (all-zero bytes == 0.0 for CGFloat, so behavior is preserved). The tests are meaningful (they exercise 10k-char / 500-column inputs that would have stressed the old VLAs and assert correct output counts).

However, two of the three sites introduce a leak regression on exception unwind paths that did not exist with the VLA (VLA storage is auto-reclaimed when the stack unwinds; malloc/calloc is not).

Important -- free is skipped on @throw paths (Sites 2 and 3)

Site 2 changeFont() and Site 3 makeTable: only free on the normal return path. Both have reachable @throw paths between allocation and free:

• changeFont -> styleCharacter -> getDefaultStyle can @throw IllegalCharacter (MTTypesetter.m:226) and the style switch can @throw Invalid style (MTTypesetter.m:419).
• makeTable: calls typesetCells: (and makeRowWithColumns:) which call createLineForMathList: -> changeFont for cell contents, so the same exceptions propagate up through makeTable:, skipping free(columnWidths).

With the VLA this was harmless; now an IllegalCharacter / Invalid style exception leaks charBuffer / columnWidths. Recommend wrapping the body in @try { ... } @finally { free(ptr); } (or freeing immediately before each rethrow). Site 1 (mathListForCharacters:) has no throw between malloc and free, so it is fine.

Minor -- no NULL check on malloc/calloc

None of the three sites check the allocation result. On failure, the subsequent getCharacters: (Sites 1/2) writes through a NULL pointer -> crash rather than graceful handling. Low practical risk at these sizes, but worth a guard given this is a hardening PR.

Minor / note -- unchecked size multiplication in the malloc sites

malloc(sizeof(unichar) * (size_t)len) (Sites 1/2) is not overflow-safe in principle, though not exploitable on 64-bit since NSString length is bounded well below the overflow threshold. Site 3 correctly uses calloc(numColumns, sizeof(CGFloat)), whose two-argument form is overflow-checked by spec -- the malloc sites do not get that protection. Acceptable as-is, noted for completeness.

Not approving

Holding approval pending the exception-path leak fix (Important).

[kostub]

Addressed the blocking exception-path leak in db50120.

Important (blocking) — free skipped on @throw paths (Sites 2 & 3): Fixed. Wrapped the alloc..use regions in @try { ... } @finally { free(ptr); }:

• changeFont() — charBuffer: styleCharacter can @throw IllegalCharacter (MTTypesetter.m:226) / Invalid style (:419) from inside the loop; now freed on unwind.
• makeTable: — columnWidths: same exceptions propagate up via typesetCells: -> createLineForMathList: -> changeFont; tableDisplay hoisted out of the @Try and returned after the @finally.

Site 1 (mathListForCharacters:) left as straight-line free — confirmed no @throw between malloc and free (atomForCharacter: doesn't throw), matching your note.

Minor — NULL check: Added NSAssert/NSCAssert non-NULL guards at all three sites. Note these compile out under NS_BLOCK_ASSERTIONS, so they're a debug-time guard rather than a release-time graceful fallback; given the low practical risk you flagged at these sizes, I kept it minimal rather than changing the return contract on alloc failure.

Minor — unchecked size multiplication: The two malloc sites use NSString.length (bounded well below the 64-bit overflow threshold for sizeof(unichar) * len), and Site 3 keeps the overflow-checked two-arg calloc. Left as-is per your note.

Re: the gemini-code-assist suggestion of a stack/heap hybrid — that's a perf optimization orthogonal to the correctness/leak fix; not adopting it here to keep this change focused on the security hardening + leak.

swift build and swift test green (295 tests, 0 failures).