Skip to content

Stop json/plain from allocating arbitrary classes (0.0.7) - #359

Merged
d2army merged 12 commits into
transfers-masterfrom
ian-yap/secbugs-174-fail-closed-json-plain
Sep 3, 2026
Merged

Stop json/plain from allocating arbitrary classes (0.0.7)#359
d2army merged 12 commits into
transfers-masterfrom
ian-yap/secbugs-174-fail-closed-json-plain

Conversation

@d2army

@d2army d2army commented Aug 27, 2026

Copy link
Copy Markdown

Summary

Oj object mode can allocate any Ruby class named in a json/plain payload. This PR allowlists what may round-trip, then loads the original bytes so existing encoded history still works. Encoding name stays json/plain. Version is 0.0.7.

deserialize runs three steps, in this order:

  1. Streaming walk (Oj::Saj): reject duplicate object keys and nesting deeper than 512. Saj has to run first. JSON.parse keeps the last duplicate key; Oj binds ^o on the first, so a discarded object or array can still allocate a class.
  2. Allowlist on the JSON.parse tree: only loaded Temporal:: types, loaded ::Request / ::Response, loaded Exception subclasses, Date-like types, anonymous Structs, and Temporal::JSON.allow_class.
  3. Oj.load of the original bytes: keep Time (^t) and symbol keys.

Do not tighten ^c. It returns a Class object (not an instance) when an exception ivar holds a class.

How to review

Start here:

  1. lib/temporal/json.rb — whole guard. File header is the map. Then PayloadStructureValidator#note (why keys are recorded on hash and array start), then allowed_* / allow_class.
  2. lib/temporal/errors.rbJSONDisallowedClassError.
  3. lib/temporal/concerns/input_deserializer.rb — also rescue JSON::ParserError so newline-split Go-client input still works. Do not swallow JSONDisallowedClassError.

Then, only if you are checking packaging or CI:

  • temporal.gemspecgoogle-protobuf ~> 3.25 because generated stubs do not load on protobuf 4.
  • .github/workflows/tests.yml — CI now runs on transfers-master PRs (it did not before). docker compose v2, wait-for-7233, and image pins are so example CI actually starts Temporal.
  • spec/unit/lib/temporal/json_spec.rb — first-party round-trips that broke on the first fail-closed pass, then attack/duplicate-key regressions.

Test plan

  • bundle exec rspec spec/unit (643 examples, 0 failures)
  • Gadgets (Gem::Requirement, Kernel) and duplicate keys (both value orderings, including ^u) raise before Oj.load
  • Nesting 512 accepted, 513 rejected; autoload-pending constants are not loaded
  • Request/Response, exceptions with backtraces, Date, anonymous Struct, json/plain encoding name
  • Go-client newline input still deserializes
  • GitHub Actions test_gem and test_examples
  • After publish: consumers can pin temporal-ruby 0.0.7 instead of a git SHA

@d2army d2army changed the title Fail-closed json/plain Oj deserialize (SECBUGS-174) Fail-closed json/plain Oj deserialize (0.0.7) Sep 3, 2026
@coinbase coinbase deleted a comment from linear Bot Sep 3, 2026
@d2army d2army changed the title Fail-closed json/plain Oj deserialize (0.0.7) Stop json/plain from allocating arbitrary classes (0.0.7) Sep 3, 2026
d2army and others added 12 commits September 2, 2026 22:30
Reject arbitrary ^o instantiation while keeping Request/Response
payloads and error serialization v2 working.

Co-authored-by: Cursor <cursoragent@cursor.com>
Unconstrained bundle install was pulling protobuf 4, which cannot load the generated stubs, and ubuntu-latest no longer ships docker-compose.

Co-authored-by: Cursor <cursoragent@cursor.com>
docker compose now starts, but auto-setup is not ready after a 10s sleep, so register_namespace hits connection refused.

Co-authored-by: Cursor <cursoragent@cursor.com>
auto-setup: latest never bound gRPC with Cassandra 3.11 on GitHub runners. Dump compose logs if the wait still times out.
Co-authored-by: Cursor <cursoragent@cursor.com>
Example workflows return Metadata::Workflow and Struct.new results. Those are first-party Oj encodings, not gadget classes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Oj puts ^o Thread::Backtrace on ~bt_locations. Rename the unit file to json_spec.rb so CI actually runs it.

Co-authored-by: Cursor <cursoragent@cursor.com>
…trips.

Oj::Saj rejects duplicate hash keys so JSON.parse and Oj.load cannot disagree
on ^o. Allow Date/DateTime/Rational via ^O, widen InputDeserializer rescue
for JSON::ParserError, and add regression specs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Record hash keys on Saj container entry, cap nesting in Saj and JSON.parse,
walk directive trees iteratively, skip autoload stubs during validation, and
declare google-protobuf ~> 3.25. Add regression specs for container duplicate
keys and depth-5000 rejection.

Co-authored-by: Cursor <cursoragent@cursor.com>
Remove the 256-character hash key limit pending replay validation, freeze
error message constants, document ^c usage for Exception ivars, and add
scalar-first duplicate, sibling-key control, and MAX_NESTING boundary tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
google-protobuf ~> 3.25 is declared in temporal.gemspec; drop the extra
root and examples Gemfile constraints. Remove the depth-5000 example now
covered by the MAX_NESTING boundary tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
Spell out what still works and what can break. Comment why Saj records
container keys, why parse order is Saj then JSON then Oj, why ^c stays
broad, and why InputDeserializer also rescues JSON::ParserError.

Co-authored-by: Cursor <cursoragent@cursor.com>
File-level deserialize order on json.rb; why-comments on the new error,
protobuf 3 pin, CI trigger, compose image pins, and spec groupings.

Co-authored-by: Cursor <cursoragent@cursor.com>
@d2army
d2army force-pushed the ian-yap/secbugs-174-fail-closed-json-plain branch from 8703d60 to 58d40e5 Compare September 3, 2026 05:33
@d2army
d2army merged commit 51ec070 into transfers-master Sep 3, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant