From f7e077f85e61fc0b7381963eda0ceb0e457546b5 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 2840781dde..22c47e4eab 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -7,7 +7,6 @@ raise e end -require "active_support/core_ext/marshal" require "active_support/core_ext/array/extract_options" module ActiveSupport @@ -28,14 +27,6 @@ class MemCacheStore < Store # 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) @@ -189,9 +180,8 @@ def normalize_key(key, options) 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 0e472f5a1a..b1c4ee1d56 100644 --- a/activesupport/test/cache/stores/mem_cache_store_test.rb +++ b/activesupport/test/cache/stores/mem_cache_store_test.rb @@ -67,7 +67,7 @@ def test_raw_values_with_marshal cache = ActiveSupport::Cache.lookup_store(*store, raw: true) cache.clear cache.write("foo", Marshal.dump([])) - assert_equal [], cache.read("foo") + assert_equal Marshal.dump([]), cache.read("foo") end def test_local_cache_raw_values @@ -100,7 +100,7 @@ def test_local_cache_raw_values_with_marshal cache.clear cache.with_local_cache do cache.write("foo", Marshal.dump([])) - assert_equal [], cache.read("foo") + assert_equal Marshal.dump([]), cache.read("foo") end end -- GitLab