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]
This commit is contained in:
Dylan Thacker-Smith 2018-09-22 17:57:58 -04:00 committed by Aaron Patterson
parent 0b2ca61b55
commit 0a4cf42311
No known key found for this signature in database
GPG Key ID: 953170BCB4FFAFC6
2 changed files with 4 additions and 14 deletions

View File

@ -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

View File

@ -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