NNUE: Bound network description size

[overcharged-coder]

Limits the NNUE network description length accepted from the file header before resizing the description string.

The description size is stored in the NNUE header and read from the network file. Previously, this value was used directly to resize the description string, so a malformed file could request an excessively large allocation before the loader failed.

This patch adds a conservative upper bound for the description field and applies the same bound when writing network headers.

Verification:

• make -j build passed.

[github-actions]

(execution 27439275413 / attempt 1)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 71e95912-ef10-47c4-8be4-4b0eb07502ef

📥 Commits

Reviewing files that changed from the base of the PR and between 86f1df7 and 476a7a4.

📒 Files selected for processing (1)

• src/nnue/network.cpp

---

📝 Walkthrough

Walkthrough

This change adds description size validation to the NNUE network I/O layer. A new internal constant Detail::MaxDescriptionSize (1 MiB) bounds description payload sizes. The Network::read_header() method now validates the stream state and enforces the size limit, then conditionally reads description bytes only when size is non-zero. The Network::write_header() method rejects oversized descriptions and conditionally writes description bytes only when the description is non-empty, preventing unsafe access.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ddobbelaere]

I don't think this is an issue that needs to be fixed. The file is supposed to be sane and worst case the kernel will throw us out for OOM reasons.

EDIT: just wondering, I see you guarding for size 0. Do we currently bail out cleanly in that case, in master?

[anematode]

I don't think we need to be sanitizing network data

Could be a fun CTF pwn challenge ;)

[overcharged-coder]

I agree this is not an issue for valid/sane network files, and I’m not trying to argue that normal network data needs heavy validation.

The reason I thought this was worth guarding is narrower: the value comes from serialized file metadata and is used as an allocation size before the loader has any chance to reject the file. So a corrupt header can make the process attempt an arbitrary-sized std::string resize before stream.read() fails.

I agree the worst case is “just” process termination / OOM rather than memory corruption, but avoiding an avoidable OOM path seems like a reasonable cheap check at the file boundary. The limit is intentionally far above any realistic description size, so it should not affect valid networks.

On the size-0 question: I don’t think master bails out for size 0. It resizes the string to 0 and then calls read(..., 0), which should effectively be a no-op and leave the stream OK. The if (size) in this patch was mostly to avoid forming a writable pointer through index 0 on an empty string and to make the zero-length case explicit. I’m happy to drop that part if it distracts from the main allocation bound.

If the project preference is that NNUE files are assumed sane and we intentionally avoid even small validation checks here, I’m fine closing this PR.