[jira] [Commented] (SLING-8879) Make JSONObject#toString and XSSAPI#encodeForJSString both safe and correct for pasting into a javascript string literal

["Ilguiz Latypov (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-8879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16988929#comment-16988929 ] 

Ilguiz Latypov commented on SLING-8879:
---------------------------------------

https://github.com/apache/sling-org-apache-sling-xss/blob/a827eb7/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java#L412


> 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
>            Priority: Minor
>
> 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)