activesupport: Avoid Marshal.load on raw cache value in MemCacheStore

dylanahsmith · tenderlove · commit 0a7ce52486ad · 2020-05-15T15:35:48.000-07:00
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]
diff --git 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)
@@ -194,9 +185,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
@@ -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
 
