Skip to content

Commit

Permalink
Merge pull request from GHSA-pghf-347x-c2gj
Browse files Browse the repository at this point in the history
* Fix CVE-2021-30459 by creating signature from all data fields.

Create a signature based on all fields in the form and attach
to validate that the data being sent back is what the server
generated initially.

Change the hashing algorithm to SHA256

Force the values to a string for signing.

Remove hashing mechanism from forms.

Support sha1 algorithm for django < 3.1

* Bump version to 3.2.1
  • Loading branch information
tim-schilling committed Apr 14, 2021
1 parent 8b280e1 commit 38e1bd7
Show file tree
Hide file tree
Showing 16 changed files with 212 additions and 138 deletions.
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion debug_toolbar/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
20 changes: 19 additions & 1 deletion debug_toolbar/decorators.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import functools

from django.http import Http404
from django.http import Http404, HttpResponseBadRequest


def require_show_toolbar(view):
Expand All @@ -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
53 changes: 53 additions & 0 deletions debug_toolbar/forms.py
Original file line number Diff line number Diff line change
@@ -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})
)
30 changes: 0 additions & 30 deletions debug_toolbar/panels/history/forms.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
11 changes: 8 additions & 3 deletions debug_toolbar/panels/history/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -76,16 +77,20 @@ 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(
self.template,
{
"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
),
},
)
Expand Down
12 changes: 7 additions & 5 deletions debug_toolbar/panels/history/views.py
Original file line number Diff line number Diff line change
@@ -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"]
Expand All @@ -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 = []
Expand Down
31 changes: 0 additions & 31 deletions debug_toolbar/panels/sql/forms.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -21,25 +16,13 @@ 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()
raw_sql = forms.CharField()
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"]
Expand All @@ -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"]]
Expand Down
5 changes: 4 additions & 1 deletion debug_toolbar/panels/sql/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 10 additions & 7 deletions debug_toolbar/panels/sql/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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"]
Expand Down Expand Up @@ -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"]
Expand Down
7 changes: 7 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading

0 comments on commit 38e1bd7

Please sign in to comment.