Fixed #29708 -- Deprecated PickleSerializer.

9 files changed, 48 insertions, 26 deletions

diff --git a/django/contrib/sessions/serializers.py b/django/contrib/sessions/serializers.py
index 5b6c343daa..6a9452bba0 100644
--- a/django/contrib/sessions/serializers.py
+++ b/django/contrib/sessions/serializers.py
@@ -1,3 +1,4 @@
+# RemovedInDjango50Warning.
 from django.core.serializers.base import (
     PickleSerializer as BasePickleSerializer,
 )
diff --git a/django/core/serializers/base.py b/django/core/serializers/base.py
index 3bd492ec09..45c43a77d6 100644
--- a/django/core/serializers/base.py
+++ b/django/core/serializers/base.py
@@ -2,10 +2,12 @@
 Module for abstract serializer/unserializer base classes.
 """
 import pickle
+import warnings
 from io import StringIO
 
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import models
+from django.utils.deprecation import RemovedInDjango50Warning
 
 DEFER_FIELD = object()
 
@@ -16,6 +18,11 @@ class PickleSerializer:
     cache backends.
     """
     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
 
     def dumps(self, obj):
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 4ab7a4135d..23fa942ef9 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -79,6 +79,8 @@ details on these changes.
   ``SimpleTestCase.assertFormError()`` and ``assertFormsetError()`` will be
   removed.
 
+* ``django.contrib.sessions.serializers.PickleSerializer`` will be removed.
+
 .. _deprecation-removed-in-4.1:
 
 4.1
diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt
index 6c3d7530a1..e65f92b4ff 100644
--- a/docs/ref/settings.txt
+++ b/docs/ref/settings.txt
@@ -3384,14 +3384,11 @@ sessions won't be created, even if this setting is active.
 Default: ``'django.contrib.sessions.serializers.JSONSerializer'``
 
 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'``
 
-See :ref:`session_serialization` for details, including a warning regarding
-possible remote code execution when using
-:class:`~django.contrib.sessions.serializers.PickleSerializer`.
+See :ref:`session_serialization` for details.
 
 Sites
 =====
diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt
index 3b36e7ea54..32e816b940 100644
--- a/docs/releases/4.1.txt
+++ b/docs/releases/4.1.txt
@@ -403,6 +403,9 @@ Miscellaneous
 * The ``exc_info`` argument of the undocumented
   ``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
 =======================
 
diff --git a/docs/topics/http/sessions.txt b/docs/topics/http/sessions.txt
index ce92af7b71..20502208a8 100644
--- a/docs/topics/http/sessions.txt
+++ b/docs/topics/http/sessions.txt
@@ -124,7 +124,7 @@ and the :setting:`SECRET_KEY` setting.
 .. warning::
 
     **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.**
 
     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
     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:
 
 Write your own serializer
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Note that unlike :class:`~django.contrib.sessions.serializers.PickleSerializer`,
-the :class:`~django.contrib.sessions.serializers.JSONSerializer` cannot handle
-arbitrary Python data types. As is often the case, there is a trade-off between
-convenience and security. If you wish to store more advanced data types
-including ``datetime`` and ``Decimal`` in JSON backed sessions, you will need
-to write a custom serializer (or convert such values to a JSON serializable
-object before storing them in ``request.session``). While serializing these
-values is often straightforward
+Note that the :class:`~django.contrib.sessions.serializers.JSONSerializer`
+cannot handle arbitrary Python data types. As is often the case, there is a
+trade-off between convenience and security. If you wish to store more advanced
+data types including ``datetime`` and ``Decimal`` in JSON backed sessions, you
+will need to write a custom serializer (or convert such values to a JSON
+serializable object before storing them in ``request.session``). While
+serializing these values is often straightforward
 (: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
 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
-  :class:`~django.contrib.sessions.serializers.JSONSerializer` or any
-  picklable Python object when using
-  :class:`~django.contrib.sessions.serializers.PickleSerializer`. See the
-  :mod:`pickle` module for more information.
+  :class:`~django.contrib.sessions.serializers.JSONSerializer`.
 
 * Session data is stored in a database table named ``django_session`` .
 
diff --git a/tests/defer_regress/tests.py b/tests/defer_regress/tests.py
index ccd6556c80..300524a151 100644
--- a/tests/defer_regress/tests.py
+++ b/tests/defer_regress/tests.py
@@ -4,7 +4,8 @@ from django.contrib.contenttypes.models import ContentType
 from django.contrib.sessions.backends.db import SessionStore
 from django.db import models
 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 (
     Base, Child, Derived, Feature, Item, ItemAndSimpleItem, Leaf, Location,
@@ -91,6 +92,7 @@ class DeferRegressionTest(TestCase):
             list(SimpleItem.objects.annotate(Count('feature')).only('name')),
             list)
 
+    @ignore_warnings(category=RemovedInDjango50Warning)
     @override_settings(SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer')
     def test_ticket_12163(self):
         # Test for #12163 - Pickling error saving session with unsaved model
diff --git a/tests/serializers/tests.py b/tests/serializers/tests.py
index 4bd6acd982..1b375a4a1e 100644
--- a/tests/serializers/tests.py
+++ b/tests/serializers/tests.py
@@ -10,7 +10,8 @@ from django.core.serializers.base import PickleSerializer, ProgressBar
 from django.db import connection, transaction
 from django.http import HttpResponse
 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 (
     Actor, Article, Author, AuthorProfile, BaseModel, Category, Child,
@@ -420,6 +421,7 @@ class SerializersTransactionTestBase:
 
 
 class PickleSerializerTests(SimpleTestCase):
+    @ignore_warnings(category=RemovedInDjango50Warning)
     def test_serializer_protocol(self):
         serializer = PickleSerializer(protocol=3)
         self.assertEqual(serializer.protocol, 3)
@@ -427,12 +429,21 @@ class PickleSerializerTests(SimpleTestCase):
         serializer = PickleSerializer()
         self.assertEqual(serializer.protocol, pickle.HIGHEST_PROTOCOL)
 
+    @ignore_warnings(category=RemovedInDjango50Warning)
     def test_serializer_loads_dumps(self):
         serializer = PickleSerializer()
         test_data = 'test data'
         dump = serializer.dumps(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=()):
     """
diff --git a/tests/sessions_tests/tests.py b/tests/sessions_tests/tests.py
index 816777bc97..367c40fcc3 100644
--- a/tests/sessions_tests/tests.py
+++ b/tests/sessions_tests/tests.py
@@ -7,6 +7,7 @@ import unittest
 from datetime import timedelta
 from http import cookies
 from pathlib import Path
+from unittest import mock
 
 from django.conf import settings
 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.models import Session
-from django.contrib.sessions.serializers import (
-    JSONSerializer, PickleSerializer,
-)
+from django.contrib.sessions.serializers import JSONSerializer
 from django.core import management
 from django.core.cache import caches
 from django.core.cache.backends.base import InvalidCacheBackendError
@@ -880,9 +879,8 @@ class CookieSessionTests(SessionTestsMixin, SimpleTestCase):
         # by creating a new session
         self.assertEqual(self.session.serializer, JSONSerializer)
         self.session.save()
-
-        self.session.serializer = PickleSerializer
-        self.session.load()
+        with mock.patch('django.core.signing.loads', side_effect=ValueError):
+            self.session.load()
 
     @unittest.skip("Cookie backend doesn't have an external store to create records in.")
     def test_session_load_does_not_create_record(self):