Skip to content

Limit JSON nesting depth and update the vendored minimal-json - #81

Open
henryx wants to merge 14 commits into
jopenlibs:masterfrom
henryx:master
Open

henryx wants to merge 14 commits into
jopenlibs:masterfrom
henryx:master

Conversation

@henryx

@henryx henryx commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A JSON value nested deeply enough makes the vendored parser recurse until the reading thread runs
out of stack. The resulting StackOverflowError is an Error, not an Exception, so it escapes
the catch blocks in the response classes and in the calling application, and kills the thread.
Because the value is stored in Vault, every later read of that path fails the same way.

This PR fixes it in two layers:

  1. The driver checks the nesting depth before parsing. Every response from Vault goes through
    a new internal SafeJson.parse(), which rejects JSON nested deeper than 500 levels. The check
    is a loop, so it works on any thread stack size. A response nested too deeply is ignored, like
    any other malformed response.
  2. The vendored minimal-json is updated to the latest upstream version. Among other fixes,
    upstream's parser rejects JSON nested deeper than 1000 levels with a ParseException. This
    limit protects applications that call Json.parse() directly, but the parser still recurses
    up to that depth, which is too deep for small thread stacks (512 KB or less). The driver's own
    check covers that case.

Vault 1.20 and later reject values nested deeper than 298 levels, so the 500 limit does not affect
valid data. Older Vault versions accept deeper values, so the fix matters most for them.

Changes

Update vendored JSON library (9c314d5)

  • io.github.jopenlibs.vault.json is replaced with the latest upstream minimal-json, repackaged.
    New public types: JsonHandler, Location, and a public streaming JsonParser
  • Upstream tests replace the old vendored tests, with imports adapted to the project's checkstyle
    rules
  • Callers of methods the driver had added to its old copy are adapted:
    • Logical.delete(), unDelete() and destroy() use Json.array(versions) instead of
      JsonObject.add(String, int[]). The request body is the same JSON array ([1,2] instead of
      [1, 2])
    • MountPayload uses private addIfNotNull() helpers instead of JsonObject.addIfNotNull()
    • MountResponse uses private getters returning null for missing members, instead of the
      one-argument JsonObject.getString(), getInt() and getBoolean()

Add nesting depth check (adac3f2)

  • New package io.github.jopenlibs.vault.internal, not exported by module-info.java:
    • SafeJson.parse(String) scans the text once, counting [/{ against ]/} and skipping
      string contents (escaped quotes included). It throws JsonNestingException above
      MAX_NESTING_DEPTH (500), and otherwise calls Json.parse(). Other syntax errors are left to
      the parser
    • JsonNestingException is an unchecked exception
  • SafeJsonTests (12 tests):
    • Arrays and objects at the limit are accepted; one level more is rejected, for both and for a
      mix of them
    • Brackets inside strings are ignored, and escaped quotes and escaped backslashes are handled
    • AuthResponse ignores a deeply nested body
    • A 9,998-level payload read through LogicalResponse on a thread with a 256 KB stack no longer
      kills the thread. Parsing the same payload with plain Json.parse() on that thread raises
      StackOverflowError, so this test covers the bug

Use the check everywhere (d75a1fd)

  • All 10 Json.parse() calls in LogicalResponse, AuthResponse, WrapResponse,
    LookupResponse, SealResponse, HealthResponse and VaultImpl use SafeJson.parse()
  • AuthResponse, WrapResponse, LookupResponse and SealResponse catch
    ParseException | JsonNestingException. LogicalResponse and HealthResponse already catch
    Exception; in VaultImpl the exception becomes a VaultException through the existing
    catch (RuntimeException)
  • README: Security entry in the Unreleased changelog

Breaking changes

The vendored package is public API (LogicalResponse.getDataObject(), UnwrapResponse.getData(),
Auth.wrap(), Wrapping.wrap(), MountPayload, LogicalUtilities). Upstream minimal-json does
not have some methods that the driver had added to its old copy, so code calling them no longer
compiles, and code compiled against 6.2.3 fails at runtime with NoSuchMethodError:

  • JsonObject.getString(String), getInt(String), getLong(String), getFloat(String),
    getDouble(String) and getBoolean(String), which returned null for missing members. The
    two-argument versions with a default value are still available
  • JsonObject.addIfNotNull(String, Object) and JsonObject.add(String, int[])
  • Json.value(int[])

Also, Json.array() now returns JsonArray instead of JsonValue. Source code still compiles,
but code compiled against 6.2.3 fails at runtime with NoSuchMethodError.

These need a changelog entry, and either a major release or restoring the methods.

Testing

  • Unit tests: 656 of 657 pass, including the 12 new SafeJsonTests
  • Integration tests: all 72 pass against a real Vault (4 skipped by the existing tests)
  • Checkstyle passes

Thanks

Special thanks to Echo to help to find this problem

@henryx henryx self-assigned this Oct 15, 2026
@henryx henryx added bug Something isn't working next-major-version Issue can be made in the next major version labels Oct 15, 2026
@henryx henryx added this to the 7.0.0 milestone Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working next-major-version Issue can be made in the next major version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant