Skip to content
Permalink
Browse files Browse the repository at this point in the history
General: fix critical views that can be subject of XSS attacks
The views were returning not escaped content which is unsafe

Thanks to Daniel Elkabes who reported the issue
  • Loading branch information
chessbr committed Jul 7, 2021
1 parent b4ac2fd commit 75714c3
Show file tree
Hide file tree
Showing 10 changed files with 28 additions and 11 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

List all changes after the last release here (newer on top). Each change on a separate bullet point line

### Fixed

- General: fix critical vulnerability on views that were returning not escaped content making it open to XSS attacks

### Changed

- Admin: hide email template button based on permission
- Reports: improve log when an importer fails

## [2.10.8] - 2021-06-30

### Changed
Expand Down
3 changes: 2 additions & 1 deletion shuup/admin/utils/urls.py
Expand Up @@ -16,6 +16,7 @@
from django.core.exceptions import ImproperlyConfigured
from django.http.response import HttpResponseForbidden
from django.utils.encoding import force_str, force_text
from django.utils.html import escape
from django.utils.http import urlencode
from django.utils.translation import ugettext_lazy as _

Expand Down Expand Up @@ -64,7 +65,7 @@ def _get_unauth_response(self, request, reason):
# Instead of redirecting to the login page, let the user know what's wrong with
# a helpful link.
raise (
Problem(_("Can't view this page. %(reason)s") % {"reason": reason}).with_link(
Problem(_("Can't view this page. %(reason)s") % {"reason": escape(reason)}).with_link(
url=resp.url, title=_("Log in with different credentials...")
)
)
Expand Down
3 changes: 2 additions & 1 deletion shuup/core/basket/command_dispatcher.py
Expand Up @@ -11,6 +11,7 @@
from django.core.exceptions import ValidationError
from django.http import HttpResponseRedirect, JsonResponse
from django.shortcuts import redirect
from django.utils.html import escape
from django.utils.translation import ugettext_lazy as _

from shuup.apps.provides import get_provide_objects
Expand Down Expand Up @@ -68,7 +69,7 @@ def handle(self, command, kwargs=None):
try:
handler = self.get_command_handler(command)
if not handler or not callable(handler):
raise Problem(_("Error! Invalid command `%s`.") % command)
raise Problem(_("Error! Invalid command `%s`.") % escape(command))
kwargs.pop("csrfmiddlewaretoken", None) # The CSRF token should never be passed as a kwarg
kwargs.pop("command", None) # Nor the command
kwargs.update(request=self.request, basket=self.basket)
Expand Down
3 changes: 2 additions & 1 deletion shuup/front/checkout/_process.py
Expand Up @@ -9,6 +9,7 @@
from collections import OrderedDict
from django.core.exceptions import ImproperlyConfigured
from django.http.response import Http404
from django.utils.html import escape

from shuup.front.basket import get_basket
from shuup.utils.django_compat import reverse
Expand Down Expand Up @@ -75,7 +76,7 @@ def get_current_phase(self, requested_phase_identifier):
return phase
if not phase.should_skip() and not phase.is_valid(): # A past phase is not valid, that's the current one
return phase
raise Http404("Error! Phase with identifier `%s` not found." % requested_phase_identifier) # pragma: no cover
raise Http404("Error! Phase with identifier `%s` not found." % escape(requested_phase_identifier))

def _get_next_phase(self, phases, current_phase, target_phase):
found = False
Expand Down
3 changes: 2 additions & 1 deletion shuup/front/urls.py
Expand Up @@ -10,6 +10,7 @@
from django.conf.urls import url
from django.contrib.auth.decorators import login_required
from django.http.response import HttpResponse
from django.utils.html import escape
from django.views.decorators.csrf import csrf_exempt
from django.views.i18n import set_language
from itertools import chain
Expand Down Expand Up @@ -37,7 +38,7 @@


def _not_here_yet(request, *args, **kwargs):
return HttpResponse("Not here yet: %s (%r, %r)" % (request.path, args, kwargs), status=410)
return HttpResponse("Not here yet: %s (%r, %r)" % (request.path, escape(args), escape(kwargs)), status=410)


# Use a different js catalog function in front urlpatterns to prevent forcing
Expand Down
7 changes: 4 additions & 3 deletions shuup/utils/excs.py
@@ -1,4 +1,5 @@
from django.core.exceptions import ValidationError
from django.utils.html import escape

from shuup.utils.django_compat import force_text

Expand Down Expand Up @@ -61,10 +62,10 @@ def extract_messages(obj_list):
for obj in obj_list:
if isinstance(obj, ValidationError):
for msg in obj.messages:
yield force_text(msg)
yield escape(force_text(msg))
continue
if isinstance(obj, Exception):
if len(obj.args):
yield force_text(obj.args[0])
yield escape(force_text(obj.args[0]))
continue
yield force_text(obj)
yield escape(force_text(obj))
2 changes: 1 addition & 1 deletion shuup/xtheme/urls.py
Expand Up @@ -19,7 +19,7 @@

urlpatterns = [
url(r"^xtheme/editor/$", EditorView.as_view(), name="xtheme_editor"),
url(r"^xtheme/(?P<view>.+)/*$", extra_view_dispatch, name="xtheme_extra_view"),
url(r"^xtheme/(?P<view>.+)/?$", extra_view_dispatch, name="xtheme_extra_view"),
url(r"^xtheme/$", command_dispatch, name="xtheme"),
url(
r"^xtheme-prod-hl/(?P<plugin_type>.*)/(?P<cutoff_days>\d+)/(?P<count>\d+)/(?P<cache_timeout>\d+)/$",
Expand Down
3 changes: 2 additions & 1 deletion shuup/xtheme/views/command.py
Expand Up @@ -6,6 +6,7 @@
# This source code is licensed under the OSL-3.0 license found in the
# LICENSE file in the root directory of this source tree.
from django.http.response import HttpResponseRedirect
from django.utils.html import escape

from shuup.utils.excs import Problem
from shuup.xtheme.editing import set_edit_mode
Expand Down Expand Up @@ -42,4 +43,4 @@ def command_dispatch(request):
response = handle_command(request, command)
if response:
return response
raise Problem("Error! Unknown command: `%r`" % command)
raise Problem("Error! Unknown command: `%r`" % escape(command))
3 changes: 2 additions & 1 deletion shuup/xtheme/views/editor.py
Expand Up @@ -8,6 +8,7 @@
import json
from django.http.response import HttpResponse, HttpResponseRedirect
from django.middleware.csrf import get_token
from django.utils.html import escape
from django.utils.http import urlencode
from django.utils.translation import ugettext_lazy as _
from django.views.generic import TemplateView
Expand Down Expand Up @@ -70,7 +71,7 @@ def post(self, request, *args, **kwargs): # doccov: ignore
if command:
dispatcher = getattr(self, "dispatch_%s" % command, None)
if not callable(dispatcher):
raise Problem(_("Unknown command: `%s`.") % command)
raise Problem(_("Unknown command: `%s`.") % escape(command))
dispatch_kwargs = dict(request.POST.items())
rv = dispatcher(**dispatch_kwargs)
if rv:
Expand Down
3 changes: 2 additions & 1 deletion shuup/xtheme/views/extra.py
Expand Up @@ -8,6 +8,7 @@
from django.core.exceptions import ImproperlyConfigured
from django.core.signals import setting_changed
from django.http.response import HttpResponseNotFound
from django.utils.html import escape

from shuup.xtheme._theme import get_current_theme

Expand Down Expand Up @@ -56,6 +57,6 @@ def extra_view_dispatch(request, view):
theme = getattr(request, "theme", None) or get_current_theme(request.shop)
view_func = get_view_by_name(theme, view)
if not view_func:
msg = "Error! %s/%s: Not found." % (getattr(theme, "identifier", None), view)
msg = "Error! %s/%s: Not found." % (getattr(theme, "identifier", None), escape(view))
return HttpResponseNotFound(msg)
return view_func(request)

0 comments on commit 75714c3

Please sign in to comment.