Fixed #29708 -- Deprecated PickleSerializer.

This commit is contained in:
Adam Johnson 2020-01-30 09:28:32 +00:00 committed by Mariusz Felisiak
parent c920387fab
commit 45a42aabfa
9 changed files with 48 additions and 26 deletions

View File

@ -1,3 +1,4 @@
# RemovedInDjango50Warning.
from django.core.serializers.base import ( from django.core.serializers.base import (
PickleSerializer as BasePickleSerializer, PickleSerializer as BasePickleSerializer,
) )

View File

@ -2,10 +2,12 @@
Module for abstract serializer/unserializer base classes. Module for abstract serializer/unserializer base classes.
""" """
import pickle import pickle
import warnings
from io import StringIO from io import StringIO
from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ObjectDoesNotExist
from django.db import models from django.db import models
from django.utils.deprecation import RemovedInDjango50Warning
DEFER_FIELD = object() DEFER_FIELD = object()
@ -16,6 +18,11 @@ class PickleSerializer:
cache backends. cache backends.
""" """
def __init__(self, protocol=None): def __init__(self, protocol=None):
warnings.warn(
'PickleSerializer is deprecated due to its security risk. Use '
'JSONSerializer instead.',
RemovedInDjango50Warning,
)
self.protocol = pickle.HIGHEST_PROTOCOL if protocol is None else protocol self.protocol = pickle.HIGHEST_PROTOCOL if protocol is None else protocol
def dumps(self, obj): def dumps(self, obj):

View File

@ -79,6 +79,8 @@ details on these changes.
``SimpleTestCase.assertFormError()`` and ``assertFormsetError()`` will be ``SimpleTestCase.assertFormError()`` and ``assertFormsetError()`` will be
removed. removed.
* ``django.contrib.sessions.serializers.PickleSerializer`` will be removed.
.. _deprecation-removed-in-4.1: .. _deprecation-removed-in-4.1:
4.1 4.1

View File

@ -3384,14 +3384,11 @@ sessions won't be created, even if this setting is active.
Default: ``'django.contrib.sessions.serializers.JSONSerializer'`` Default: ``'django.contrib.sessions.serializers.JSONSerializer'``
Full import path of a serializer class to use for serializing session data. Full import path of a serializer class to use for serializing session data.
Included serializers are: Included serializer is:
* ``'django.contrib.sessions.serializers.PickleSerializer'``
* ``'django.contrib.sessions.serializers.JSONSerializer'`` * ``'django.contrib.sessions.serializers.JSONSerializer'``
See :ref:`session_serialization` for details, including a warning regarding See :ref:`session_serialization` for details.
possible remote code execution when using
:class:`~django.contrib.sessions.serializers.PickleSerializer`.
Sites Sites
===== =====

View File

@ -403,6 +403,9 @@ Miscellaneous
* The ``exc_info`` argument of the undocumented * The ``exc_info`` argument of the undocumented
``django.utils.log.log_response()`` function is replaced by ``exception``. ``django.utils.log.log_response()`` function is replaced by ``exception``.
* ``django.contrib.sessions.serializers.PickleSerializer`` is deprecated due to
the risk of remote code execution.
Features removed in 4.1 Features removed in 4.1
======================= =======================

View File

@ -124,7 +124,7 @@ and the :setting:`SECRET_KEY` setting.
.. warning:: .. warning::
**If the SECRET_KEY is not kept secret and you are using the** **If the SECRET_KEY is not kept secret and you are using the**
:class:`~django.contrib.sessions.serializers.PickleSerializer`, **this can ``django.contrib.sessions.serializers.PickleSerializer``, **this can
lead to arbitrary remote code execution.** lead to arbitrary remote code execution.**
An attacker in possession of the :setting:`SECRET_KEY` can not only An attacker in possession of the :setting:`SECRET_KEY` can not only
@ -362,19 +362,23 @@ Bundled serializers
remote code execution vulnerability if :setting:`SECRET_KEY` becomes known remote code execution vulnerability if :setting:`SECRET_KEY` becomes known
by an attacker. by an attacker.
.. deprecated:: 4.1
Due to the risk of remote code execution, this serializer is deprecated
and will be removed in Django 5.0.
.. _custom-serializers: .. _custom-serializers:
Write your own serializer Write your own serializer
~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~
Note that unlike :class:`~django.contrib.sessions.serializers.PickleSerializer`, Note that the :class:`~django.contrib.sessions.serializers.JSONSerializer`
the :class:`~django.contrib.sessions.serializers.JSONSerializer` cannot handle cannot handle arbitrary Python data types. As is often the case, there is a
arbitrary Python data types. As is often the case, there is a trade-off between trade-off between convenience and security. If you wish to store more advanced
convenience and security. If you wish to store more advanced data types data types including ``datetime`` and ``Decimal`` in JSON backed sessions, you
including ``datetime`` and ``Decimal`` in JSON backed sessions, you will need will need to write a custom serializer (or convert such values to a JSON
to write a custom serializer (or convert such values to a JSON serializable serializable object before storing them in ``request.session``). While
object before storing them in ``request.session``). While serializing these serializing these values is often straightforward
values is often straightforward
(:class:`~django.core.serializers.json.DjangoJSONEncoder` may be helpful), (:class:`~django.core.serializers.json.DjangoJSONEncoder` may be helpful),
writing a decoder that can reliably get back the same thing that you put in is writing a decoder that can reliably get back the same thing that you put in is
more fragile. For example, you run the risk of returning a ``datetime`` that more fragile. For example, you run the risk of returning a ``datetime`` that
@ -664,10 +668,7 @@ Technical details
================= =================
* The session dictionary accepts any :mod:`json` serializable value when using * The session dictionary accepts any :mod:`json` serializable value when using
:class:`~django.contrib.sessions.serializers.JSONSerializer` or any :class:`~django.contrib.sessions.serializers.JSONSerializer`.
picklable Python object when using
:class:`~django.contrib.sessions.serializers.PickleSerializer`. See the
:mod:`pickle` module for more information.
* Session data is stored in a database table named ``django_session`` . * Session data is stored in a database table named ``django_session`` .

View File

@ -4,7 +4,8 @@ from django.contrib.contenttypes.models import ContentType
from django.contrib.sessions.backends.db import SessionStore from django.contrib.sessions.backends.db import SessionStore
from django.db import models from django.db import models
from django.db.models import Count from django.db.models import Count
from django.test import TestCase, override_settings from django.test import TestCase, ignore_warnings, override_settings
from django.utils.deprecation import RemovedInDjango50Warning
from .models import ( from .models import (
Base, Child, Derived, Feature, Item, ItemAndSimpleItem, Leaf, Location, Base, Child, Derived, Feature, Item, ItemAndSimpleItem, Leaf, Location,
@ -91,6 +92,7 @@ class DeferRegressionTest(TestCase):
list(SimpleItem.objects.annotate(Count('feature')).only('name')), list(SimpleItem.objects.annotate(Count('feature')).only('name')),
list) list)
@ignore_warnings(category=RemovedInDjango50Warning)
@override_settings(SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer') @override_settings(SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer')
def test_ticket_12163(self): def test_ticket_12163(self):
# Test for #12163 - Pickling error saving session with unsaved model # Test for #12163 - Pickling error saving session with unsaved model

View File

@ -10,7 +10,8 @@ from django.core.serializers.base import PickleSerializer, ProgressBar
from django.db import connection, transaction from django.db import connection, transaction
from django.http import HttpResponse from django.http import HttpResponse
from django.test import SimpleTestCase, override_settings, skipUnlessDBFeature from django.test import SimpleTestCase, override_settings, skipUnlessDBFeature
from django.test.utils import Approximate from django.test.utils import Approximate, ignore_warnings
from django.utils.deprecation import RemovedInDjango50Warning
from .models import ( from .models import (
Actor, Article, Author, AuthorProfile, BaseModel, Category, Child, Actor, Article, Author, AuthorProfile, BaseModel, Category, Child,
@ -420,6 +421,7 @@ class SerializersTransactionTestBase:
class PickleSerializerTests(SimpleTestCase): class PickleSerializerTests(SimpleTestCase):
@ignore_warnings(category=RemovedInDjango50Warning)
def test_serializer_protocol(self): def test_serializer_protocol(self):
serializer = PickleSerializer(protocol=3) serializer = PickleSerializer(protocol=3)
self.assertEqual(serializer.protocol, 3) self.assertEqual(serializer.protocol, 3)
@ -427,12 +429,21 @@ class PickleSerializerTests(SimpleTestCase):
serializer = PickleSerializer() serializer = PickleSerializer()
self.assertEqual(serializer.protocol, pickle.HIGHEST_PROTOCOL) self.assertEqual(serializer.protocol, pickle.HIGHEST_PROTOCOL)
@ignore_warnings(category=RemovedInDjango50Warning)
def test_serializer_loads_dumps(self): def test_serializer_loads_dumps(self):
serializer = PickleSerializer() serializer = PickleSerializer()
test_data = 'test data' test_data = 'test data'
dump = serializer.dumps(test_data) dump = serializer.dumps(test_data)
self.assertEqual(serializer.loads(dump), test_data) self.assertEqual(serializer.loads(dump), test_data)
def test_serializer_warning(self):
msg = (
'PickleSerializer is deprecated due to its security risk. Use '
'JSONSerializer instead.'
)
with self.assertRaisesMessage(RemovedInDjango50Warning, msg):
PickleSerializer()
def register_tests(test_class, method_name, test_func, exclude=()): def register_tests(test_class, method_name, test_func, exclude=()):
""" """

View File

@ -7,6 +7,7 @@ import unittest
from datetime import timedelta from datetime import timedelta
from http import cookies from http import cookies
from pathlib import Path from pathlib import Path
from unittest import mock
from django.conf import settings from django.conf import settings
from django.contrib.sessions.backends.base import UpdateError from django.contrib.sessions.backends.base import UpdateError
@ -24,9 +25,7 @@ from django.contrib.sessions.exceptions import (
) )
from django.contrib.sessions.middleware import SessionMiddleware from django.contrib.sessions.middleware import SessionMiddleware
from django.contrib.sessions.models import Session from django.contrib.sessions.models import Session
from django.contrib.sessions.serializers import ( from django.contrib.sessions.serializers import JSONSerializer
JSONSerializer, PickleSerializer,
)
from django.core import management from django.core import management
from django.core.cache import caches from django.core.cache import caches
from django.core.cache.backends.base import InvalidCacheBackendError from django.core.cache.backends.base import InvalidCacheBackendError
@ -880,9 +879,8 @@ class CookieSessionTests(SessionTestsMixin, SimpleTestCase):
# by creating a new session # by creating a new session
self.assertEqual(self.session.serializer, JSONSerializer) self.assertEqual(self.session.serializer, JSONSerializer)
self.session.save() self.session.save()
with mock.patch('django.core.signing.loads', side_effect=ValueError):
self.session.serializer = PickleSerializer self.session.load()
self.session.load()
@unittest.skip("Cookie backend doesn't have an external store to create records in.") @unittest.skip("Cookie backend doesn't have an external store to create records in.")
def test_session_load_does_not_create_record(self): def test_session_load_does_not_create_record(self):