Skip to content

test[translation-nmtcpp]: bounded FuzzTest for GGML weight-header parsers - #4594

Open
jpgaribotti wants to merge 1 commit into
tetherto:mainfrom
jpgaribotti:feat/translation-nmtcpp-weight-header-fuzz
Open

jpgaribotti wants to merge 1 commit into
tetherto:mainfrom
jpgaribotti:feat/translation-nmtcpp-weight-header-fuzz

Conversation

@jpgaribotti

Copy link
Copy Markdown
Contributor

🎯 What problem does this PR solve?

  • #4590 closed a stack smash when an untrusted NMT model set n_dims > 4. Nearby length-prefixed fields (nmtReadBoundedString, nmtReadCount, tensor type) have the same shape.
  • Bounded FuzzTest coverage over those parsers would have caught the n_dims=8 case, and keeps the reject paths reachable on every Linux C++ CI run.

📝 How does it solve it?

  • Extract the weight-header parsers into nmt_loader_parse.cpp so the fuzz target compiles them without @qvac/fabric (full ASan + LeakSanitizer).
  • Add nmt-weight-header-fuzz with seeded FUZZ_TESTs, including the n_dims=8 crash from Fix NMT ndims overflow crash #4590.
  • Wire fuzz* scripts, the vcpkg fuzz feature, and Linux npm run fuzz after the coverage harvest in reusable-cpp-tests-translation-nmtcpp.yml.
  • Patch bump @qvac/translation-nmtcpp 0.16.40.16.5. No public addon API changes.

🧪 How was it tested?

  • Follows the classification-ggml Phase 0 template (docs/architecture/ADDON-FUZZING.md).
  • Bounded mode is what Linux C++ CI will run (npm run fuzz:build + npm run fuzz:run).
  • on-pr-* is pull_request_target and reads workflow YAML from main, so the new fuzz steps run after this lands, not on this PR itself.

The n_dims stack overflow in tetherto#4590 was a length-field parse bug that unit
tests only catch once the case is known. A no-fabric FuzzTest over the
extracted parsers reaches n_dims>4, oversized names, and short reads on
every Linux C++ CI run.
@github-actions

Copy link
Copy Markdown
Contributor

Review Status

Current Status: ❌ PENDING
Approvals so far: none

Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member.

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.

1 participant