Thanks to Moritz Naumann who found the issues and wrote a very useful report, I fixed a number of Cross Site Scripting vulnerabilities on https://debtags.debian.org.
The core of the issue was code like this in a Django view:
def pkginfo_view(request, name):
pkg = bmodels.Package.by_name(name)
if pkg is None:
return http.HttpResponseNotFound("Package %s was not found" % name)
# …
The default content-type of HttpResponseNotFound
is text/html
, and the
string passed is the raw HTML with clearly no escaping, so this allows
injection of arbitrary HTML/<script>
code in the name
variable.
I was so used to Django doing proper auto-escaping that I missed this place in which it can't do that.
There are various things that can be improved in that code.
One could introduce
escaping
(and while one's at it, migrate the old %
to format
):
from django.utils.html import escape
def pkginfo_view(request, name):
pkg = bmodels.Package.by_name(name)
if pkg is None:
return http.HttpResponseNotFound("Package {} was not found".format(escape(name)))
# …
Alternatively, set content_type
to text/plain
:
def pkginfo_view(request, name):
pkg = bmodels.Package.by_name(name)
if pkg is None:
return http.HttpResponseNotFound("Package {} was not found".format(name), content_type="text/plain")
# …
Even better, raise Http404:
from django.utils.html import escape
def pkginfo_view(request, name):
pkg = bmodels.Package.by_name(name)
if pkg is None:
raise Http404(f"Package {name} was not found")
# …
Even better, use standard shortcuts and model functions if possible:
from django.shortcuts import get_object_or_404
def pkginfo_view(request, name):
pkg = get_object_or_404(bmodels.Package, name=name)
# …
And finally, though not security related, it's about time to switch to class-based views:
class PkgInfo(TemplateView):
template_name = "reports/package.html"
def get_context_data(self, **kw):
ctx = super().get_context_data(**kw)
ctx["pkg"] = get_object_or_404(bmodels.Package, name=self.kwargs["name"])
# …
return ctx
I proceeded with a review of the other Django sites I maintain in case I reproduced this mistake also there.