From 0a4cf42311b35d46718ab3bbfac3d51568bdf23d Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Sat, 22 Sep 2018 17:57:58 -0400 Subject: [PATCH] activesupport: Avoid Marshal.load on raw cache value in MemCacheStore Dalli is already being used for marshalling, so we should also rely on it for unmarshalling. Since Dalli tags the cache value as marshalled it can avoid unmarshalling a raw string which might have come from an untrusted source. [CVE-2020-8165] --- .../lib/active_support/cache/mem_cache_store.rb | 14 ++------------ .../test/cache/stores/mem_cache_store_test.rb | 4 ++-- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index bebe87b019..6b90d91ed3 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -8,7 +8,6 @@ rescue LoadError => e end require "active_support/core_ext/enumerable" -require "active_support/core_ext/marshal" require "active_support/core_ext/array/extract_options" module ActiveSupport @@ -29,14 +28,6 @@ module ActiveSupport # Provide support for raw values in the local cache strategy. module LocalCacheWithRaw # :nodoc: private - def read_entry(key, **options) - entry = super - if options[:raw] && local_cache && entry - entry = deserialize_entry(entry.value) - end - entry - end - def write_entry(key, entry, **options) if options[:raw] && local_cache raw_entry = Entry.new(entry.value.to_s) @@ -195,9 +186,8 @@ module ActiveSupport key end - def deserialize_entry(raw_value) - if raw_value - entry = Marshal.load(raw_value) rescue raw_value + def deserialize_entry(entry) + if entry entry.is_a?(Entry) ? entry : Entry.new(entry) end end diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb index f578714cdd..96d2680f54 100644 --- a/activesupport/test/cache/stores/mem_cache_store_test.rb +++ b/activesupport/test/cache/stores/mem_cache_store_test.rb @@ -79,7 +79,7 @@ class MemCacheStoreTest < ActiveSupport::TestCase def test_raw_values_with_marshal cache = lookup_store(raw: true) cache.write("foo", Marshal.dump([])) - assert_equal [], cache.read("foo") + assert_equal Marshal.dump([]), cache.read("foo") end def test_local_cache_raw_values @@ -108,7 +108,7 @@ class MemCacheStoreTest < ActiveSupport::TestCase cache = lookup_store(raw: true) cache.with_local_cache do cache.write("foo", Marshal.dump([])) - assert_equal [], cache.read("foo") + assert_equal Marshal.dump([]), cache.read("foo") end end