Fail-closed json/plain Oj deserialize (SECBUGS-174)

.github/workflows/tests.yml

@@ -2,9 +2,9 @@ name: Tests
 
 on:
   push:
-    branches: [ "master" ]
+    branches: [ "master", "transfers-master" ]
   pull_request:
-    branches: [ "master" ]
+    branches: [ "master", "transfers-master" ]
 
 jobs:
   test_gem:
@@ -37,7 +37,7 @@ jobs:
 
       - name: Start dependencies
         run: |
-          docker-compose \
+          docker compose \
             -f examples/docker-compose.yml \
             up -d
 
@@ -50,13 +50,29 @@ jobs:
         run: |
           cd examples && bundle install --path vendor/bundle
 
-      - name: Wait for dependencies to settle
+      - name: Wait for Temporal
         run: |
-          sleep 10
+          if timeout 300 bash -c 'until echo >/dev/tcp/127.0.0.1/7233; do sleep 2; done'; then
+            exit 0
+          fi
+          echo "Temporal did not listen on 7233"
+          docker compose -f examples/docker-compose.yml ps -a
+          docker compose -f examples/docker-compose.yml logs --no-color --tail 400
+          exit 1
 
       - name: Register namespace
         run: |
-          cd examples && bin/register_namespace ruby-samples
+          cd examples
+          for i in $(seq 1 30); do
+            if bin/register_namespace ruby-samples; then
+              exit 0
+            fi
+            echo "register_namespace failed, retry ${i}"
+            sleep 5
+          done
+          docker compose -f docker-compose.yml ps
+          docker compose -f docker-compose.yml logs --tail 200
+          exit 1
 
       - name: Wait for namespace to settle
         run: |
@@ -78,6 +94,10 @@ jobs:
         run: |
           cd examples && bin/worker &
 
+      - name: Wait for workers
+        run: |
+          sleep 10
+
       - name: Run RSpec
         env:
           USE_ERROR_SERIALIZATION_V2: 1

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.0.7
+
+- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), Oj Exception backtrace types (`Thread::Backtrace`), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged (SECBUGS-174).
+
 ## 0.0.6
 
 - Defer gRPC loading via autoload for fork safety

Gemfile

@@ -1,3 +1,6 @@
 source 'https://rubygems.org'
 
 gemspec
+
+# Generated lib/gen stubs use protobuf 3 DescriptorPool#build, which protobuf 4 removed.
+gem 'google-protobuf', '~> 3.25.0'

examples/Gemfile

@@ -1,6 +1,7 @@
 source 'https://rubygems.org'
 
 gem 'temporal-ruby', path: '../'
+gem 'google-protobuf', '~> 3.25.0'
 
 gem 'dry-types', '>= 1.2.0'
 gem 'dry-struct', '~> 1.1.1'

examples/docker-compose.yml

@@ -2,7 +2,7 @@ version: '3.5'
 
 services:
   temporal:
-    image: temporalio/auto-setup:latest
+    image: temporalio/auto-setup:1.22.4
     ports:
      - "7233:7233"
     environment:
@@ -14,7 +14,7 @@ services:
       - cassandra
 
   temporal-web:
-    image: temporalio/web:latest
+    image: temporalio/web:1.15.0
     environment:
       - "TEMPORAL_GRPC_ENDPOINT=temporal:7233"
     ports:
@@ -23,6 +23,6 @@ services:
       - temporal
 
   cassandra:
-    image: cassandra:3.11
+    image: cassandra:3.11.16
     ports:
       - "9042:9042"

lib/temporal/errors.rb

@@ -2,6 +2,8 @@ module Temporal
   # Superclass for all Temporal errors
   class Error < StandardError; end
 
+  class JSONDisallowedClassError < Error; end
+
   # Superclass for errors specific to Temporal worker itself
   class InternalError < Error; end
 

lib/temporal/json.rb

@@ -1,5 +1,8 @@
 # Helper class for serializing/deserializing JSON
+require 'json'
 require 'oj'
+require 'set'
+require 'temporal/errors'
 
 module Temporal
   module JSON
@@ -9,12 +12,155 @@ module JSON
       float_precision: 0
     }.freeze
 
+    # ^o / ^O allocate instances. ^c / ^C look up Class objects (error serialization v2).
+    INSTANCE_DIRECTIVE_KEYS = %w[^o ^O].freeze
+    CLASS_REFERENCE_DIRECTIVE_KEYS = %w[^c ^C].freeze
+    STRUCT_DIRECTIVE_KEYS = %w[^u].freeze
+    ALLOWED_CLASS_SUFFIXES = %w[::Request ::Response].freeze
+    # Oj dumps these on a raised Exception (~bt_locations). They are not gadget classes.
+    ALLOWED_STDLIB_CLASSES = %w[
+      Thread::Backtrace
+      Thread::Backtrace::Location
+    ].freeze
+
+    ALLOWED_CLASSES = Set.new
+    ALLOWED_CLASSES_MUTEX = Mutex.new
+    private_constant :ALLOWED_CLASSES, :ALLOWED_CLASSES_MUTEX
+
     def self.serialize(value)
       Oj.dump(value, OJ_OPTIONS)
     end
 
+    # SECBUGS-174: Oj mode: :object instantiates any constant named in ^o.
+    # Walk a JSON.parse tree first and only then Oj.load the original bytes so
+    # symbol keys and ^t stay compatible with encode.
     def self.deserialize(value)
-      Oj.load(value.to_s, OJ_OPTIONS)
+      return nil if value.nil?
+
+      raw = value.to_s
+      return nil if raw.empty?
+
+      assert_safe!(::JSON.parse(raw))
+      Oj.load(raw, OJ_OPTIONS)
+    end
+
+    def self.allow_class(name)
+      with_allowed_classes { |set| set.add(name.to_s) }
+      name.to_s
+    end
+
+    def self.allowed_class_names
+      with_allowed_classes(&:dup)
+    end
+
+    def self.with_allowed_classes
+      ALLOWED_CLASSES_MUTEX.synchronize { yield ALLOWED_CLASSES }
+    end
+    private_class_method :with_allowed_classes
+
+    def self.assert_safe!(obj)
+      case obj
+      when Hash
+        STRUCT_DIRECTIVE_KEYS.each do |key|
+          next unless obj.key?(key)
+          next if anonymous_struct_directive?(obj[key])
+
+          name = class_name_from_directive(obj[key])
+          unless name && allowed_struct_class?(name)
+            raise Temporal::JSONDisallowedClassError,
+                  "json/plain payload requested disallowed class #{obj[key].inspect}"
+          end
+        end
+
+        INSTANCE_DIRECTIVE_KEYS.each do |key|
+          next unless obj.key?(key)
+
+          name = class_name_from_directive(obj[key])
+          unless name && allowed_instance_class?(name)
+            raise Temporal::JSONDisallowedClassError,
+                  "json/plain payload requested disallowed class #{obj[key].inspect}"
+          end
+        end
+
+        CLASS_REFERENCE_DIRECTIVE_KEYS.each do |key|
+          next unless obj.key?(key)
+
+          name = class_name_from_directive(obj[key])
+          unless name && allowed_class_reference?(name)
+            raise Temporal::JSONDisallowedClassError,
+                  "json/plain payload requested disallowed class #{obj[key].inspect}"
+          end
+        end
+
+        obj.each_value { |v| assert_safe!(v) }
+      when Array
+        obj.each { |v| assert_safe!(v) }
+      end
+    end
+    private_class_method :assert_safe!
+
+    def self.class_name_from_directive(value)
+      case value
+      when String
+        value
+      when Array
+        value.first if value.first.is_a?(String)
+      end
+    end
+    private_class_method :class_name_from_directive
+
+    def self.registered_class?(name)
+      with_allowed_classes { |set| set.include?(name) }
+    end
+    private_class_method :registered_class?
+
+    def self.allowed_instance_class?(name)
+      return true if registered_class?(name)
+      return true if library_class?(name)
+      return true if ALLOWED_STDLIB_CLASSES.include?(name) && resolve_constant(name)
+      return true if ALLOWED_CLASS_SUFFIXES.any? { |suffix| name.end_with?(suffix) } && resolve_constant(name)
+
+      klass = resolve_constant(name)
+      klass.is_a?(Class) && klass <= Exception
+    end
+    private_class_method :allowed_instance_class?
+
+    def self.allowed_struct_class?(name)
+      registered_class?(name) || library_class?(name)
+    end
+    private_class_method :allowed_struct_class?
+
+    def self.library_class?(name)
+      name.start_with?('Temporal::') && resolve_constant(name).is_a?(Class)
+    end
+    private_class_method :library_class?
+
+    # Oj encodes Struct.new(:a, :b).new(...) as ^u with member names, not a class.
+    def self.anonymous_struct_directive?(value)
+      value.is_a?(Array) && value.first.is_a?(Array) && value.first.all? { |member| member.is_a?(String) }
+    end
+    private_class_method :anonymous_struct_directive?
+
+    def self.allowed_class_reference?(name)
+      return true if registered_class?(name)
+
+      resolve_constant(name).is_a?(Module)
+    end
+    private_class_method :allowed_class_reference?
+
+    def self.resolve_constant(name)
+      parts = name.split('::')
+      parts.shift if parts.first.empty?
+      return nil if parts.empty?
+
+      parts.reduce(Object) do |mod, part|
+        return nil unless mod.is_a?(Module) && mod.const_defined?(part, false)
+
+        mod.const_get(part, false)
+      end
+    rescue NameError
+      nil
     end
+    private_class_method :resolve_constant
   end
 end

lib/temporal/version.rb

@@ -1,3 +1,3 @@
 module Temporal
-  VERSION = '0.0.6'.freeze
+  VERSION = '0.0.7'.freeze
 end

spec/unit/lib/temporal/connection/converter/payload/json_spec.rb

@@ -3,6 +3,10 @@
 describe Temporal::Connection::Converter::Payload::JSON do
   subject { described_class.new }
 
+  it 'keeps the json/plain encoding name' do
+    expect(subject.encoding).to eq('json/plain')
+  end
+
   describe 'round trip' do
     it 'safely handles non-ASCII encodable UTF characters' do
       input = { 'one' => 'one', two: :two, ':three' => '☻' }