A potential improvement in the Django template filter `escapejs`

[Phithon]

Hi, I recently encountered a problem that is related to Django, I think I should report and discuss with you.

As I know, escapejs is a helpful template filter which can protect us from XSS attack in Javascript string like this:

<script>
    function example() {
        query = '{{ request.GET.q | escapejs }}';
    }
</script>

But there is usually a situation, as time goes on, someone thinks this function is not used anymore and comments it out:

<script>
    /*
    function example() {
        query = '{{ request.GET.q | escapejs }}';
    }
    */
</script>

Oops, A XSS vulnerability is introduced. The attackers can trigger arbitrary javascript execution by the request http://example.com/?q=*/(alert(1))/*

It's not a Django security bug but I also suggest making the / and * escaped in the escapejs filter.

[Phithon]

PR: ​https://github.com/django/django/pull/12717

[Claude Paroz]

I'm afraid this must be classified in programming errors, for which Django cannot help. The escapejs filter has absolutely no way to detect if it is used in commented code. In your example, it only receives the request.GET.q content, not the code around it. I'm suggesting to won't fix that issue, but I'll let a second triager confirm it.

Also please note that security issues should be privately sent to security@… and never on public issue trackers.
​https://docs.djangoproject.com/en/stable/internals/security/#reporting-security-issues

[Phithon]

Replying to Claude Paroz:

I'm afraid this must be classified in programming errors, for which Django cannot help. The escapejs filter has absolutely no way to detect if it is used in commented code. In your example, it only receives the request.GET.q content, not the code around it. I'm suggesting to won't fix that issue, but I'll let a second triager confirm it.

Also please note that security issues should be privately sent to security@… and never on public issue trackers.
​https://docs.djangoproject.com/en/stable/internals/security/#reporting-security-issues

I don't think it is a security issue, it is a security-related proposal that works for some bad programming practices. Only two characters added in escapejs process, the cost is very small.

Here is a reference to Secure by default policy ​https://en.wikipedia.org/wiki/Secure_by_default.

[Claude Paroz]

Are you aware that in the example you provided, the \* and */ parts are not passed to escapejs?

[Mariusz Felisiak]

Replying to Claude Paroz:

Are you aware that in the example you provided, the \* and */ parts are not passed to escapejs?

They pass q=*/(alert(1))/* so in a clever way we end with a valid JS:

<script>
    /*
    function example() {
        query = '*/(alert(1))/*';
    }
    */
</script>

Replying to Phithon:

Oops, A XSS vulnerability is introduced. The attackers can trigger arbitrary javascript execution by the request

escapejs does not make the string safe for use in HTML or JavaScript template literals, see ​documentation. It only protects you from syntax errors when using templates to generate JavaScript/JSON. I agree with Claude.

See related #29055.

[Phithon]

Hi, thanks for your reply.

In my option, the document and former ticket are talking about "HTML or JavaScript template literals", but the current context is "JavaScript string", seems like a bit different.

Anyway, this ticket can help people who have bad programming practices although it was closed.