Ilguiz Latypov created SLING-8879:
-------------------------------------

             Summary: Make JSONObject#toString and XSSAPI#encodeForJSString 
both safe and correct for pasting into a javascript string literal
                 Key: SLING-8879
                 URL: https://issues.apache.org/jira/browse/SLING-8879
             Project: Sling
          Issue Type: Bug
          Components: XSS Protection API
            Reporter: Ilguiz Latypov


The current implementation risks exceptions in strict javascript engines.

|return source == null ? null : 
Encode.forJavaScript(source).replace("\\-", "\\u002D");|

Substitutes on top of the encoder's result with the intent to correct the 
encoder are near-sighted (i.e. suffer from the context-free approach). If 
{{source}} had a backslash followed by a dash, i.e. {{raw \-}}, the 
{{forJavaScript}} call would properly change the backslash into 2 backslashes 
{{raw \\-}} (this would result in the javascript engine turning 
the string literal back to {{raw \-}}). But the subsequent 
{{replace}} call will destroy the context of the second backslash, turning the 
string into {{raw \\u002D}} which would turn to {{raw \u002D}} 
in the javascript engine's variable.

I argue for dropping the {{.replace()}} call (aiming at disabling malicious 
comment injection) here and encoding the opening angle bracket {{raw <}} as 
{{raw &#x5C;u003C}} in the {{Encode.forJavaScript}} implementation. This will 
protect against both the malicious comment injection and the injection of 
closing script tags {{raw &#x3C;&#x5C;script>}} forcing the javascript 
interpreter to drop out of the string literal context and drop out of the 
script context.

The existing prefixing of forward slashes with a backslash agrees with JSON but 
not with Javascript. It should be removed in favour of replacing just the 
opening angle bracket.
{noformat}
SingleEscapeCharacter :: one of
' " \ b f n r t v
{noformat}
[https://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals]

[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Escape_notation]

I noticed that JSONObject#toString suffers from the same idea of a 
non-universal protection of the forward slash.  I guess both XSSAPI and 
JSONObject#toString reuse the same code.




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to