Skip to content

gguf: fix GGML_PAD integer overflow via crafted general.alignment (CWE-190)#2201

Open
Gzhao-redpoint wants to merge 1 commit into
huggingface:mainfrom
Gzhao-redpoint:fix/gguf-ggml-pad-overflow
Open

gguf: fix GGML_PAD integer overflow via crafted general.alignment (CWE-190)#2201
Gzhao-redpoint wants to merge 1 commit into
huggingface:mainfrom
Gzhao-redpoint:fix/gguf-ggml-pad-overflow

Conversation

@Gzhao-redpoint
Copy link
Copy Markdown

@Gzhao-redpoint Gzhao-redpoint commented May 29, 2026

Summary

Fixes an integer overflow in GGML_PAD that can be triggered by a crafted GGUF file with general.alignment >= 2^31, bypassing the validation added in #2028.

Root Cause

JavaScript bitwise operators (&, ~) truncate operands to 32-bit signed integers. When alignment >= 2^31:

const GGML_PAD = (x, n) => (x + n - 1) & ~(n - 1);

GGML_PAD(1000, 2**31)   // → -2147483648  (should be 2147483648)
GGML_PAD(1000, 2**32-1) // → 0            (should be 4294967295)

A malicious GGUF with general.alignment = 2147483648 (UINT32) passes all existing validation (positive, integer, within safe integer range) but produces a negative tensorDataOffset.

Impact

Entry point Behavior
gguf() parser (line 509) tensorDataOffset = negative BigInt → corrupted tensor data offset
serializeGgufMetadata() (line 710) new Uint8Array(negative)RangeError (DoS)
buildGgufHeader() (line 801) Negative padLen → corrupted header or RangeError

Fix

  • Add power-of-2 validation — matching llama.cpp's gguf.cpp line 612: (alignment & (alignment - 1)) !== 0
  • Add upper bound MAX_ALIGNMENT = 2^30 — the largest power-of-2 safe for JS bitwise operations
  • Validate in all three entry points: gguf(), serializeGgufMetadata(), buildGgufHeader()

Reproduction

// Create minimal GGUF v3 with general.alignment = 2^31
const buf = new ArrayBuffer(64);
const view = new DataView(buf);
// ... magic "GGUF", version 3, 0 tensors, 1 KV entry ...
// KV: key="general.alignment", type=UINT32, value=2147483648

// Parse → tensorDataOffset will be -2147483648n (NEGATIVE)

CWE

  • CWE-190: Integer Overflow or Wraparound

Test plan

  • Verify alignment = 32, 64, 1, 2^20, 2^30 are accepted
  • Verify alignment = 2^31, 2^32-1, 0, -1, 3, 33 are rejected
  • Run existing test suite (pnpm run test in packages/gguf)

Note

Medium Risk
Targets malicious/crafted GGUF input and header rebuild paths; behavior change rejects some alignment values that previously passed basic integer checks.

Overview
Hardens GGUF alignment handling so crafted general.alignment values cannot break GGML_PAD via JavaScript’s 32-bit bitwise truncation (CWE-190).

Introduces MAX_ALIGNMENT (2³⁰) and rejects alignments that are not powers of two or exceed that cap. The same rules apply when parsing metadata in gguf() and when alignment is passed into serializeGgufMetadata() and buildGgufHeader(), closing paths that previously yielded negative tensorDataOffset, invalid buffer sizes, or corrupted headers.

Reviewed by Cursor Bugbot for commit e828ae6. Bugbot is set up for automated code reviews on this repo. Configure here.

…(CWE-190)

JavaScript bitwise operators (`&`, `~`) truncate to 32-bit signed integers.
When `general.alignment >= 2^31`, `GGML_PAD(offset, alignment)` overflows,
producing negative values that corrupt `tensorDataOffset` and cause
`RangeError` in serialization paths.

This bypasses the validation added in PR huggingface#2028 because values like
`alignment = 2147483648` pass the positive-integer check but overflow
in bitwise operations.

Fix:
- Add power-of-2 validation (matching llama.cpp `gguf.cpp` line 612)
- Add upper bound `MAX_ALIGNMENT = 2^30` (largest safe power-of-2 for
  JS bitwise ops)
- Validate in all three entry points: `gguf()`, `serializeGgufMetadata()`,
  and `buildGgufHeader()`

Co-Authored-By: Claude Opus 4.6 <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.

2 participants