diff --git a/README.rst b/README.rst index fb9180741..3a3304780 100644 --- a/README.rst +++ b/README.rst @@ -34,7 +34,7 @@ Here's a screenshot of the toolbar in action: In addition to the built-in panels, a number of third-party panels are contributed by the community. -The current stable version of the Debug Toolbar is 3.2. It works on +The current stable version of the Debug Toolbar is 3.2.1. It works on Django ≥ 2.2. Documentation, including installation and configuration instructions, is diff --git a/debug_toolbar/__init__.py b/debug_toolbar/__init__.py index ddee48b62..84eea7d7a 100644 --- a/debug_toolbar/__init__.py +++ b/debug_toolbar/__init__.py @@ -3,7 +3,7 @@ # Do not use pkg_resources to find the version but set it here directly! # see issue #1446 -VERSION = "3.2" +VERSION = "3.2.1" # Code that discovers files or modules in INSTALLED_APPS imports this module. diff --git a/debug_toolbar/decorators.py b/debug_toolbar/decorators.py index 8114b05d7..2abfb22f9 100644 --- a/debug_toolbar/decorators.py +++ b/debug_toolbar/decorators.py @@ -1,6 +1,6 @@ import functools -from django.http import Http404 +from django.http import Http404, HttpResponseBadRequest def require_show_toolbar(view): @@ -15,3 +15,21 @@ def inner(request, *args, **kwargs): return view(request, *args, **kwargs) return inner + + +def signed_data_view(view): + """Decorator that handles unpacking a signed data form""" + + @functools.wraps(view) + def inner(request, *args, **kwargs): + from debug_toolbar.forms import SignedDataForm + + data = request.GET if request.method == "GET" else request.POST + signed_form = SignedDataForm(data) + if signed_form.is_valid(): + return view( + request, *args, verified_data=signed_form.verified_data(), **kwargs + ) + return HttpResponseBadRequest("Invalid signature") + + return inner diff --git a/debug_toolbar/forms.py b/debug_toolbar/forms.py new file mode 100644 index 000000000..2b4e048b4 --- /dev/null +++ b/debug_toolbar/forms.py @@ -0,0 +1,53 @@ +import json + +from django import forms +from django.core import signing +from django.core.exceptions import ValidationError +from django.utils.encoding import force_str + + +class SignedDataForm(forms.Form): + """Helper form that wraps a form to validate its contents on post. + + class PanelForm(forms.Form): + # fields + + On render: + form = SignedDataForm(initial=PanelForm(initial=data).initial) + + On POST: + signed_form = SignedDataForm(request.POST) + if signed_form.is_valid(): + panel_form = PanelForm(signed_form.verified_data) + if panel_form.is_valid(): + # Success + Or wrap the FBV with ``debug_toolbar.decorators.signed_data_view`` + """ + + salt = "django_debug_toolbar" + signed = forms.CharField(required=True, widget=forms.HiddenInput) + + def __init__(self, *args, **kwargs): + initial = kwargs.pop("initial", None) + if initial: + initial = {"signed": self.sign(initial)} + super().__init__(*args, initial=initial, **kwargs) + + def clean_signed(self): + try: + verified = json.loads( + signing.Signer(salt=self.salt).unsign(self.cleaned_data["signed"]) + ) + return verified + except signing.BadSignature: + raise ValidationError("Bad signature") + + def verified_data(self): + return self.is_valid() and self.cleaned_data["signed"] + + @classmethod + def sign(cls, data): + items = sorted(data.items(), key=lambda item: item[0]) + return signing.Signer(salt=cls.salt).sign( + json.dumps({key: force_str(value) for key, value in items}) + ) diff --git a/debug_toolbar/panels/history/forms.py b/debug_toolbar/panels/history/forms.py index b4d9ff6f8..9280c3cc9 100644 --- a/debug_toolbar/panels/history/forms.py +++ b/debug_toolbar/panels/history/forms.py @@ -1,11 +1,4 @@ -import hashlib -import hmac - from django import forms -from django.conf import settings -from django.core.exceptions import ValidationError -from django.utils.crypto import constant_time_compare -from django.utils.encoding import force_bytes class HistoryStoreForm(forms.Form): @@ -16,26 +9,3 @@ class HistoryStoreForm(forms.Form): """ store_id = forms.CharField(widget=forms.HiddenInput()) - hash = forms.CharField(widget=forms.HiddenInput()) - - def __init__(self, *args, **kwargs): - initial = kwargs.get("initial", None) - - if initial is not None: - initial["hash"] = self.make_hash(initial) - - super().__init__(*args, **kwargs) - - @staticmethod - def make_hash(data): - m = hmac.new(key=force_bytes(settings.SECRET_KEY), digestmod=hashlib.sha1) - m.update(force_bytes(data["store_id"])) - return m.hexdigest() - - def clean_hash(self): - hash = self.cleaned_data["hash"] - - if not constant_time_compare(hash, self.make_hash(self.data)): - raise ValidationError("Tamper alert") - - return hash diff --git a/debug_toolbar/panels/history/panel.py b/debug_toolbar/panels/history/panel.py index e80d8c93a..4494bbfcd 100644 --- a/debug_toolbar/panels/history/panel.py +++ b/debug_toolbar/panels/history/panel.py @@ -8,6 +8,7 @@ from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from debug_toolbar.forms import SignedDataForm from debug_toolbar.panels import Panel from debug_toolbar.panels.history import views from debug_toolbar.panels.history.forms import HistoryStoreForm @@ -76,7 +77,9 @@ def content(self): for id, toolbar in reversed(self.toolbar._store.items()): stores[id] = { "toolbar": toolbar, - "form": HistoryStoreForm(initial={"store_id": id}), + "form": SignedDataForm( + initial=HistoryStoreForm(initial={"store_id": id}).initial + ), } return render_to_string( @@ -84,8 +87,10 @@ def content(self): { "current_store_id": self.toolbar.store_id, "stores": stores, - "refresh_form": HistoryStoreForm( - initial={"store_id": self.toolbar.store_id} + "refresh_form": SignedDataForm( + initial=HistoryStoreForm( + initial={"store_id": self.toolbar.store_id} + ).initial ), }, ) diff --git a/debug_toolbar/panels/history/views.py b/debug_toolbar/panels/history/views.py index f6f086131..b4cf8c835 100644 --- a/debug_toolbar/panels/history/views.py +++ b/debug_toolbar/panels/history/views.py @@ -1,15 +1,16 @@ from django.http import HttpResponseBadRequest, JsonResponse from django.template.loader import render_to_string -from debug_toolbar.decorators import require_show_toolbar +from debug_toolbar.decorators import require_show_toolbar, signed_data_view from debug_toolbar.panels.history.forms import HistoryStoreForm from debug_toolbar.toolbar import DebugToolbar @require_show_toolbar -def history_sidebar(request): +@signed_data_view +def history_sidebar(request, verified_data): """Returns the selected debug toolbar history snapshot.""" - form = HistoryStoreForm(request.GET) + form = HistoryStoreForm(verified_data) if form.is_valid(): store_id = form.cleaned_data["store_id"] @@ -32,9 +33,10 @@ def history_sidebar(request): @require_show_toolbar -def history_refresh(request): +@signed_data_view +def history_refresh(request, verified_data): """Returns the refreshed list of table rows for the History Panel.""" - form = HistoryStoreForm(request.GET) + form = HistoryStoreForm(verified_data) if form.is_valid(): requests = [] diff --git a/debug_toolbar/panels/sql/forms.py b/debug_toolbar/panels/sql/forms.py index 4a04ee67b..8d2709777 100644 --- a/debug_toolbar/panels/sql/forms.py +++ b/debug_toolbar/panels/sql/forms.py @@ -1,13 +1,8 @@ -import hashlib -import hmac import json from django import forms -from django.conf import settings from django.core.exceptions import ValidationError from django.db import connections -from django.utils.crypto import constant_time_compare -from django.utils.encoding import force_bytes from django.utils.functional import cached_property from debug_toolbar.panels.sql.utils import reformat_sql @@ -21,7 +16,6 @@ class SQLSelectForm(forms.Form): raw_sql: The sql statement with placeholders params: JSON encoded parameter values duration: time for SQL to execute passed in from toolbar just for redisplay - hash: the hash of (secret + sql + params) for tamper checking """ sql = forms.CharField() @@ -29,17 +23,6 @@ class SQLSelectForm(forms.Form): params = forms.CharField() alias = forms.CharField(required=False, initial="default") duration = forms.FloatField() - hash = forms.CharField() - - def __init__(self, *args, **kwargs): - initial = kwargs.get("initial") - if initial is not None: - initial["hash"] = self.make_hash(initial) - - super().__init__(*args, **kwargs) - - for name in self.fields: - self.fields[name].widget = forms.HiddenInput() def clean_raw_sql(self): value = self.cleaned_data["raw_sql"] @@ -65,23 +48,9 @@ def clean_alias(self): return value - def clean_hash(self): - hash = self.cleaned_data["hash"] - - if not constant_time_compare(hash, self.make_hash(self.data)): - raise ValidationError("Tamper alert") - - return hash - def reformat_sql(self): return reformat_sql(self.cleaned_data["sql"], with_toggle=False) - def make_hash(self, data): - m = hmac.new(key=force_bytes(settings.SECRET_KEY), digestmod=hashlib.sha1) - for item in [data["sql"], data["params"]]: - m.update(force_bytes(item)) - return m.hexdigest() - @property def connection(self): return connections[self.cleaned_data["alias"]] diff --git a/debug_toolbar/panels/sql/panel.py b/debug_toolbar/panels/sql/panel.py index 683ede292..d15d99136 100644 --- a/debug_toolbar/panels/sql/panel.py +++ b/debug_toolbar/panels/sql/panel.py @@ -7,6 +7,7 @@ from django.urls import path from django.utils.translation import gettext_lazy as _, ngettext_lazy as __ +from debug_toolbar.forms import SignedDataForm from debug_toolbar.panels import Panel from debug_toolbar.panels.sql import views from debug_toolbar.panels.sql.forms import SQLSelectForm @@ -211,7 +212,9 @@ def duplicate_key(query): query["vendor"], query["trans_status"] ) - query["form"] = SQLSelectForm(auto_id=None, initial=copy(query)) + query["form"] = SignedDataForm( + auto_id=None, initial=SQLSelectForm(initial=copy(query)).initial + ) if query["sql"]: query["sql"] = reformat_sql(query["sql"], with_toggle=True) diff --git a/debug_toolbar/panels/sql/views.py b/debug_toolbar/panels/sql/views.py index 776a4c967..3b6516b49 100644 --- a/debug_toolbar/panels/sql/views.py +++ b/debug_toolbar/panels/sql/views.py @@ -2,15 +2,16 @@ from django.template.loader import render_to_string from django.views.decorators.csrf import csrf_exempt -from debug_toolbar.decorators import require_show_toolbar +from debug_toolbar.decorators import require_show_toolbar, signed_data_view from debug_toolbar.panels.sql.forms import SQLSelectForm @csrf_exempt @require_show_toolbar -def sql_select(request): +@signed_data_view +def sql_select(request, verified_data): """Returns the output of the SQL SELECT statement""" - form = SQLSelectForm(request.POST or None) + form = SQLSelectForm(verified_data) if form.is_valid(): sql = form.cleaned_data["raw_sql"] @@ -34,9 +35,10 @@ def sql_select(request): @csrf_exempt @require_show_toolbar -def sql_explain(request): +@signed_data_view +def sql_explain(request, verified_data): """Returns the output of the SQL EXPLAIN on the given query""" - form = SQLSelectForm(request.POST or None) + form = SQLSelectForm(verified_data) if form.is_valid(): sql = form.cleaned_data["raw_sql"] @@ -69,9 +71,10 @@ def sql_explain(request): @csrf_exempt @require_show_toolbar -def sql_profile(request): +@signed_data_view +def sql_profile(request, verified_data): """Returns the output of running the SQL and getting the profiling statistics""" - form = SQLSelectForm(request.POST or None) + form = SQLSelectForm(verified_data) if form.is_valid(): sql = form.cleaned_data["raw_sql"] diff --git a/docs/changes.rst b/docs/changes.rst index bef5d1964..19aaab12d 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,6 +4,13 @@ Change log Next version ------------ + +3.2.1 (2021-04-14) +------------------ + +* Fixed SQL Injection vulnerability, CVE-2021-30459. The toolbar now + calculates a signature on all fields for the SQL select, explain, + and analyze forms. * Changed ``djdt.cookie.set()`` to set ``sameSite=Lax`` by default if callers do not provide a value. * Added ``PRETTIFY_SQL`` configuration option to support controlling diff --git a/docs/conf.py b/docs/conf.py index e29866a82..f3afd1888 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -25,7 +25,7 @@ copyright = copyright.format(datetime.date.today().year) # The full version, including alpha/beta/rc tags -release = "3.2" +release = "3.2.1" # -- General configuration --------------------------------------------------- diff --git a/setup.cfg b/setup.cfg index 93a297b31..d1df17267 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = django-debug-toolbar -version = 3.2 +version = 3.2.1 description = A configurable set of panels that display various debug information about the current request/response. long_description = file: README.rst author = Rob Hudson diff --git a/tests/panels/test_history.py b/tests/panels/test_history.py index 6d592bccf..03657a374 100644 --- a/tests/panels/test_history.py +++ b/tests/panels/test_history.py @@ -1,9 +1,7 @@ -from unittest.mock import patch - from django.test import RequestFactory, override_settings from django.urls import resolve, reverse -from debug_toolbar.panels.history.forms import HistoryStoreForm +from debug_toolbar.forms import SignedDataForm from debug_toolbar.toolbar import DebugToolbar from ..base import BaseTestCase, IntegrationTestCase @@ -83,33 +81,15 @@ def test_history_sidebar_invalid(self): response = self.client.get(reverse("djdt:history_sidebar")) self.assertEqual(response.status_code, 400) - data = { - "store_id": "foo", - "hash": "invalid", - } + data = {"signed": SignedDataForm.sign({"store_id": "foo"}) + "invalid"} response = self.client.get(reverse("djdt:history_sidebar"), data=data) self.assertEqual(response.status_code, 400) - @patch("debug_toolbar.panels.history.views.DebugToolbar.fetch") - def test_history_sidebar_hash(self, fetch): - """Validate the hashing mechanism.""" - fetch.return_value.panels = [] - data = { - "store_id": "foo", - "hash": "3280d66a3cca10098a44907c5a1fd255265eed31", - } - response = self.client.get(reverse("djdt:history_sidebar"), data=data) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.json(), {}) - def test_history_sidebar(self): """Validate the history sidebar view.""" self.client.get("/json_view/") store_id = list(DebugToolbar._store.keys())[0] - data = { - "store_id": store_id, - "hash": HistoryStoreForm.make_hash({"store_id": store_id}), - } + data = {"signed": SignedDataForm.sign({"store_id": store_id})} response = self.client.get(reverse("djdt:history_sidebar"), data=data) self.assertEqual(response.status_code, 200) self.assertEqual( @@ -130,25 +110,20 @@ def test_history_sidebar(self): }, ) - def test_history_refresh_invalid(self): + def test_history_refresh_invalid_signature(self): response = self.client.get(reverse("djdt:history_refresh")) self.assertEqual(response.status_code, 400) - data = { - "store_id": "foo", - "hash": "invalid", - } + data = {"signed": "eyJzdG9yZV9pZCI6ImZvbyIsImhhc2giOiI4YWFiMzIzZGZhODIyMW"} response = self.client.get(reverse("djdt:history_refresh"), data=data) self.assertEqual(response.status_code, 400) + self.assertEqual(b"Invalid signature", response.content) def test_history_refresh(self): """Verify refresh history response has request variables.""" data = {"foo": "bar"} self.client.get("/json_view/", data, content_type="application/json") - data = { - "store_id": "foo", - "hash": "3280d66a3cca10098a44907c5a1fd255265eed31", - } + data = {"signed": SignedDataForm.sign({"store_id": "foo"})} response = self.client.get(reverse("djdt:history_refresh"), data=data) self.assertEqual(response.status_code, 200) data = response.json() diff --git a/tests/test_forms.py b/tests/test_forms.py new file mode 100644 index 000000000..73d820fd8 --- /dev/null +++ b/tests/test_forms.py @@ -0,0 +1,56 @@ +from datetime import datetime + +import django +from django import forms +from django.test import TestCase + +from debug_toolbar.forms import SignedDataForm + +# Django 3.1 uses sha256 by default. +SIGNATURE = ( + "v02QBcJplEET6QXHNWejnRcmSENWlw6_RjxLTR7QG9g" + if django.VERSION >= (3, 1) + else "ukcAFUqYhUUnqT-LupnYoo-KvFg" +) + +DATA = {"value": "foo", "date": datetime(2020, 1, 1)} +SIGNED_DATA = f'{{"date": "2020-01-01 00:00:00", "value": "foo"}}:{SIGNATURE}' + + +class FooForm(forms.Form): + value = forms.CharField() + # Include a datetime in the tests because it's not serializable back + # to a datetime by SignedDataForm + date = forms.DateTimeField() + + +class TestSignedDataForm(TestCase): + def test_signed_data(self): + data = {"signed": SignedDataForm.sign(DATA)} + form = SignedDataForm(data=data) + self.assertTrue(form.is_valid()) + # Check the signature value + self.assertEqual(data["signed"], SIGNED_DATA) + + def test_verified_data(self): + form = SignedDataForm(data={"signed": SignedDataForm.sign(DATA)}) + self.assertEqual( + form.verified_data(), + { + "value": "foo", + "date": "2020-01-01 00:00:00", + }, + ) + # Take it back to the foo form to validate the datetime is serialized + foo_form = FooForm(data=form.verified_data()) + self.assertTrue(foo_form.is_valid()) + self.assertDictEqual(foo_form.cleaned_data, DATA) + + def test_initial_set_signed(self): + form = SignedDataForm(initial=DATA) + self.assertEqual(form.initial["signed"], SIGNED_DATA) + + def test_prevents_tampering(self): + data = {"signed": SIGNED_DATA.replace('"value": "foo"', '"value": "bar"')} + form = SignedDataForm(data=data) + self.assertFalse(form.is_valid()) diff --git a/tests/test_integration.py b/tests/test_integration.py index 59ea1c25f..ebd4f882d 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -12,6 +12,7 @@ from django.test import RequestFactory from django.test.utils import override_settings +from debug_toolbar.forms import SignedDataForm from debug_toolbar.middleware import DebugToolbarMiddleware, show_toolbar from debug_toolbar.panels import Panel from debug_toolbar.toolbar import DebugToolbar @@ -212,12 +213,15 @@ def test_template_source_checks_show_toolbar(self): def test_sql_select_checks_show_toolbar(self): url = "/__debug__/sql_select/" data = { - "sql": "SELECT * FROM auth_user", - "raw_sql": "SELECT * FROM auth_user", - "params": "{}", - "alias": "default", - "duration": "0", - "hash": "6e12daa636b8c9a8be993307135458f90a877606", + "signed": SignedDataForm.sign( + { + "sql": "SELECT * FROM auth_user", + "raw_sql": "SELECT * FROM auth_user", + "params": "{}", + "alias": "default", + "duration": "0", + } + ) } response = self.client.post(url, data) @@ -235,12 +239,15 @@ def test_sql_select_checks_show_toolbar(self): def test_sql_explain_checks_show_toolbar(self): url = "/__debug__/sql_explain/" data = { - "sql": "SELECT * FROM auth_user", - "raw_sql": "SELECT * FROM auth_user", - "params": "{}", - "alias": "default", - "duration": "0", - "hash": "6e12daa636b8c9a8be993307135458f90a877606", + "signed": SignedDataForm.sign( + { + "sql": "SELECT * FROM auth_user", + "raw_sql": "SELECT * FROM auth_user", + "params": "{}", + "alias": "default", + "duration": "0", + } + ) } response = self.client.post(url, data) @@ -265,12 +272,15 @@ def test_sql_explain_postgres_json_field(self): ) query = base_query + """ '{"foo": "bar"}'""" data = { - "sql": query, - "raw_sql": base_query + " %s", - "params": '["{\\"foo\\": \\"bar\\"}"]', - "alias": "default", - "duration": "0", - "hash": "2b7172eb2ac8e2a8d6f742f8a28342046e0d00ba", + "signed": SignedDataForm.sign( + { + "sql": query, + "raw_sql": base_query + " %s", + "params": '["{\\"foo\\": \\"bar\\"}"]', + "alias": "default", + "duration": "0", + } + ) } response = self.client.post(url, data) self.assertEqual(response.status_code, 200) @@ -287,12 +297,15 @@ def test_sql_explain_postgres_json_field(self): def test_sql_profile_checks_show_toolbar(self): url = "/__debug__/sql_profile/" data = { - "sql": "SELECT * FROM auth_user", - "raw_sql": "SELECT * FROM auth_user", - "params": "{}", - "alias": "default", - "duration": "0", - "hash": "6e12daa636b8c9a8be993307135458f90a877606", + "signed": SignedDataForm.sign( + { + "sql": "SELECT * FROM auth_user", + "raw_sql": "SELECT * FROM auth_user", + "params": "{}", + "alias": "default", + "duration": "0", + } + ) } response = self.client.post(url, data)