Skip to content

Commit

Permalink
activesupport: Avoid Marshal.load on raw cache value in MemCacheStore
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
dylanahsmith authored and tenderlove committed May 15, 2020
1 parent b3230c5 commit 0a7ce52
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 14 deletions.
14 changes: 2 additions & 12 deletions activesupport/lib/active_support/cache/mem_cache_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
raise e
end

require "active_support/core_ext/marshal"
require "active_support/core_ext/array/extract_options"

module ActiveSupport
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions activesupport/test/cache/stores/mem_cache_store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 0a7ce52

Please sign in to comment.