Stop json/plain from allocating arbitrary classes - #360
Open
d2army wants to merge 3 commits into
Open
Conversation
d2army
force-pushed
the
transfers-master
branch
from
September 3, 2026 05:43
a7fdd6c to
0aa9e10
Compare
Oj object mode can allocate any Ruby class named in a json/plain payload. Allowlist what may round-trip, reject duplicate object keys and excessive nesting, then Oj.load the original bytes so existing encoded history still works. Encoding name stays json/plain. Runtime depends on google-protobuf ~> 3.25. CI now runs on transfers-master.
d2army
force-pushed
the
ian-yap/secbugs-174-fail-closed-json-plain
branch
from
September 3, 2026 05:43
264b37e to
76bef51
Compare
allow_class previously only applied to ^o instance payloads. Classes Oj encodes as ^O were still rejected even after registration. Co-authored-by: Cursor <cursoragent@cursor.com>
Dest Transfers workers rejected AccountHistoryMetadata ^u payloads while the class was in the Set. Match registrations with == and klass.name, not only Set#include?. Co-authored-by: Cursor <cursoragent@cursor.com>
edwardzhu0211
approved these changes
Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
COR3-130 · SECBUGS-22434 · SECBUGS-174
Partner write-up: SECBUGS-174 Doc
Encoding name stays
json/plain.Problem
If an attacker can write workflow or activity input to a Temporal namespace your worker reads, they can put JSON in those payloads that names arbitrary Ruby classes. Oj will build those classes during deserialize, before activity code runs.
This is not public-internet RCE. The attacker needs write access to Temporal inputs on a namespace the worker polls.
Solution
This PR blocks that. If the class is not allowed, we raise
Temporal::JSONDisallowedClassErrorbeforeOj.load. Normal payloads still deserialize: activity::Request/::Responsetypes,Exceptionsubclasses, andTemporal::types. Apps can register more names withTemporal::JSON.allow_class(Transfers does this at boot for V1 domain objects).We did not switch to plain JSON hashes. Coinbase wire history still needs Oj object mode for Time (
^t), symbol keys, and existing^o/^ushapes. Instead,deserializeruns three steps, in this order:Oj::Saj): reject duplicate object keys and nesting deeper than 512. Saj has to run first.JSON.parsekeeps the last duplicate key; Oj binds^oon the first, so a discarded object or array can still allocate a class.JSON.parsetree: loadedTemporal::types, loaded::Request/::Response, loaded Exception subclasses,Date/DateTime/Rational,Thread::Backtrace(raised exceptions), anonymous Structs, and names registered withTemporal::JSON.allow_class. Constants that are only pendingautoloadare rejected until they are actually loaded.Oj.loadof the original bytes: keep Time (^t) and symbol keys.Do not tighten
^c. It returns a Class object (not an instance) for any already-loaded constant, which is required when an exception ivar holds a class.Consumer bake (head
2f791b6)custody/api#15261 and tx_service#9157 pin this commit on dev and staging. Do not publish 0.0.7 or merge consumer PRs to prod until gem review lands.
How to review
Start here:
lib/temporal/json.rb— whole guard. File header is the map. ThenPayloadStructureValidator#note(why keys are recorded on hash and array start), thenallowed_*/allow_class.lib/temporal/errors.rb—JSONDisallowedClassError.lib/temporal/concerns/input_deserializer.rb— also rescueJSON::ParserErrorso newline-split Go-client input still works. Do not swallowJSONDisallowedClassError.Then, only if you are checking packaging or CI:
temporal.gemspec—google-protobuf ~> 3.25because generated stubs do not load on protobuf 4..github/workflows/tests.yml— CI now runs ontransfers-masterPRs (it did not before).docker composev2, 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)Gem::Requirement,Kernel) and duplicate keys (both value orderings, including^u) raise beforeOj.loadjson/plainencoding nametest_gemandtest_exampleson this PRtemporal-ruby0.0.7 instead of a git SHA