[doringeman]

Overview

Flagging and fixing a denial of service, where a deeply nested array terminates the process, including any same-model work in-flight.

Additional information

• Build

cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLAMA_BUILD_SERVER=ON -DLLAMA_BUILD_TOOLS=ON -DLLAMA_BUILD_TESTS=OFF -DLLAMA_BUILD_EXAMPLES=OFF -DGGML_METAL=OFF
cmake --build build --target llama-server -j 16

• Run

MODEL=(~/.docker/models/bundles/sha256/0f5d7ed70618e80ad42d74852ba780f21686bb1b6ab30e1c35186f72048b8f58/model/model.gguf)
./build/bin/llama-server -m "$MODEL" --jinja --port 18080 --host 127.0.0.1 -c 2048

• Test

python3 -c "
open_br='['*3000; close_br=']'*3000
print('{\"model\":\"llama-3.2-1b\",\"tools\":'+open_br+close_br+'}')
" > /tmp/deep_payload.json

curl -s -w '\nHTTP_STATUS:%{http_code}\n' -X POST http://127.0.0.1:18080/v1/chat/completions \
  -H "Content-Type: application/json" --data-binary @/tmp/deep_payload.json

• Check results

Before:

...
HTTP_STATUS:000

# and in the server logs:
[1]    29699 bus error  ./build/bin/llama-server -m "$MODEL" --jinja --port 18080 --host 127.0.0.1 -c

Now:

...
{"error":{"code":400,"message":"request body exceeds maximum JSON nesting depth of 64","type":"invalid_request_error"}}
HTTP_STATUS:400

$ curl -s http://127.0.0.1:18080/health
{"status":"ok"}

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: YES, only to write the code.
  The root cause wasn't identified by AI. I wrote the PR description myself.

[ServeurpersoCom]

cc @ngxson

SECURITY.md puts DoS out of scope, so I read this as a plain robustness fix rather than a security one: a malformed request should never take the process down, and the bound is local with no runtime cost. Do you agree we take it on that basis, and how do you want this kind of report framed in general, since the description and the test here are worded as a security finding?

Second question, on the constant: 64 leaves around 19 levels of real nesting once the OAI envelope and the properties layers of a JSON schema are counted, which machine generated tool schemas can plausibly exceed. Any reason not to go higher, say 256, still far below where the stack actually gives out?

[ngxson]

I was able to reproduce the fix locally. Agreed that it's a robustness fix, but I think this is more likely a bug in nlohmann::json rather than from our side. Personally I'm not convinced about a string walker fix, feels like we are just reinventing a json parser on our own.

Will see if there is a better fix.

[ServeurpersoCom]

That’s the right question! Technically, it’s up to the library not to crash. But they likely think the same way: you don’t arbitrarily limit local resources.
Moreover, the library should expose a depth property/callback derived from its own parsing!

[ServeurpersoCom]

Opus 5 say :

nlohmann already exposes the depth: parser_callback_t is bool(int depth, parse_event_t, basic_json &) and the depth passed is the parser stack size, so we can reject at object_start/array_start without writing a scanner of our own. The catch is that passing a callback switches the parser to the callback DOM path, which does extra work per token, so on multi MB payloads it may well cost more than the single byte pass. Worth measuring before picking one.

On the upstream side, they already made the destructor iterative and the parser is iterative too, so what is left recursive is the copy constructor and dump(), which is where this actually dies. A depth cap at parse time does not help when the json comes from somewhere else, so the real library fix is making those two iterative, with the destructor as precedent.

[ngxson]

I mean, the first step should be to search if anyone already trying to fix upstream

nlohmann/json — issues

• #1835 — Segmentation fault in destructor in case of large inputs (CLOSED) — the bad_json_parsers report; same root cause, destruction side.
• #1788 — Add support for parse stack limiting (CLOSED, went stale) — explicit request to bound depth for untrusted input; the max_depth option was never implemented.
• #832 — Stack-overflow (OSS-Fuzz 4234) (CLOSED) — deeply nested input found by fuzzing.
• #5135 — basic_json destructor allocates memory, violating noexcept semantics (OPEN) — destructor recursion still unresolved.
• #3583 — json destructor quite slow (OPEN) — same recursive-tree issue.
• #5104 — Stack overflow in binary format parsers (CBOR/MessagePack/UBJSON/BSON) due to unbounded recursion (OPEN).
• #5317 — CBOR: nested indefinite-length strings/byte strings are accepted (OPEN) — related binary-format recursion.
• #2793 — An Ubjson Parsing Problem Can Easily Cause DDoS Attack (OPEN).
• #2598 — Unhandled exception in test-cbor.exe, stack overflow (CLOSED).

nlohmann/json — PRs

• #1436 — Prevent stackoverflow caused by recursive deconstruction (MERGED, v3.7.0) — made the text parser iterative; does not cover copy/destruct of the value type, which is where the crash still happens.

[ngxson]

since many fixes are already discussed / merged upstream, I'd suggest joining one of the discussion above

OSS maintainers in general don't want duplicated efforts, let's try to keep it low noise for both llama.cpp and upstream nlohmann::json

[doringeman]

Thank you @ServeurpersoCom @ngxson.
I opened an issue in nlohmann/json: nlohmann/json#5387.

[ServeurpersoCom]

Nice, that was the right move: nlohmann/json#1788 asked for the same thing back in 2019 and the bot closed it in 2020 without anyone acting on it, so a fresh report with a working repro is exactly what was missing.

[ServeurpersoCom]

Closing this to avoid duplicated work, since the fix belongs upstream and nlohmann/json#5389 is now open on exactly this.