grahamgilbert/Crypt-Server

500 error at /checkin (and unverified XSS)

Closed this issue · 11 comments

justforhack commented

When I access the /checkin endpoint, the server will give me a 500 error code:

HTTP/1.1 500 Internal Server Error
Content-Type: text/html; charset=utf-8
Date: Wed, 24 Jun 2020 14:53:27 GMT
Server: nginx
X-Frame-Options: SAMEORIGIN
Content-Length: 0
Connection: Close

I don't know why this happens but other endpoints still working very well, no bugs!
Also, I found an unverified XSS since I can't test it with my server when my server has been taken down for some fixes!
The following snippet is taken from /server/views.py:

        serial_link = '<a href="%s">%s</a>' % (
            reverse("server:computer_info", args=[machine["id"]]),
            machine["serial"],
        )

        computername_link = '<a href="%s">%s</a>' % (
            reverse("server:computer_info", args=[machine["id"]]),
            machine["computername"],
        )

        info_button = '<a class="btn btn-info btn-xs" href="%s">Info</a>' % (
            reverse("server:computer_info", args=[machine["id"]])
        )

        list_data = [
            serial_link,
            computername_link,
            machine["username"],
            formatted_date,
            info_button,
        ]
        return_data["data"].append(list_data)

    return JsonResponse(return_data)

The following lines show that XSS is possible if the user sets his computername to <script>alert(1)</script>:

        computername_link = '<a href="%s">%s</a>' % (
            reverse("server:computer_info", args=[machine["id"]]),
            machine["computername"],

The output will become like this:

<a href="..."><script>alert(1)</script></a>

(Sorry if bad English)

grahamgilbert commented

If you enable debug mode in your settings.py you will get the full traceback.

justforhack commented

Haven't checked yet since my server is still in fixing mode, but thanks, I will check back soon! Also, what about the bug?

For fix:

        serial_link = '<a href="%s">%s</a>' % (
            reverse("server:computer_info", args=[machine["id"]]),
[            machine["serial"].replace('<', '&lt;').replace('>', '&gt;'),
        )

        computername_link = '<a href="%s">%s</a>' % (
            reverse("server:computer_info", args=[machine["id"]]),
            machine["computername"].replace('<', '&lt;').replace('>', '&gt;'),
        )

        info_button = '<a class="btn btn-info btn-xs" href="%s">Info</a>' % (
            reverse("server:computer_info", args=[machine["id"]])
        )

        list_data = [
            serial_link,
            computername_link,
            machine["username"],
            formatted_date,
            info_button,
        ]
        return_data["data"].append(list_data)

    return JsonResponse(return_data)

It would be better if you create a function just for filtering unsafe characters.

justforhack commented

Hi, can you also give me a full list of Crypt Server endpoints? Thanks

grahamgilbert commented

I feel like there should be a built in method in Django to do this, but I have zero time to spend on this right now.

grahamgilbert commented

Yes now I’ve woken up this should just be put in the base template. An easy PR for your first https://docs.djangoproject.com/en/1.10/ref/templates/builtins/#autoescape

grahamgilbert commented

And what about the 500 error? This sounds like it should be easy enough for you to replicate?

grahamgilbert commented

Yeah, maybe, since I am going to re-build and re-run the server (still using Crypt Server).

I feel like there should be a built in method in Django to do this, but I have zero time to spend on this right now.

Why you don't have time?

Because I have a day job and open source doesn’t pay the bills.

KevinHock commented

I totally empathize with not having time to work on OSS -- I'm in the same boat.

Was this issue, the XSS, fixed? Just trying to follow along from reading this issue and that part isn't clear to me. I could probably make a PR if it is not fixed.

grahamgilbert commented

The XSS was unverified and I wasn’t given enough information to reproduce.

KevinHock commented

Makes sense, I'd do the same in your situation, thanks Graham!!