Fixed XSS issue on debtags.debian.org

pdo debian eng debtags

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.