[jira] [Comment Edited] (SLING-8879) Make JSONObject#toString and XSSAPI#encodeForJSString both safe and correct for pasting into a javascript string literal
[
https://issues.apache.org/jira/browse/SLING-8879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16989033#comment-16989033
]
Ilguiz Latypov edited comment on SLING-8879 at 1/31/20 9:24 PM:
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML
or XHTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the double quotes {{raw }} against terminating the string,
* the escape character {{raw }} against breaking the context of escaping
other characters,
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} against the mandatory HTML entity decoding in
XHTML (but not HTML) documents.
was (Author: ilatypov):
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML
or XHTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} against the mandatory HTML entity decoding in
XHTML (but not HTML) documents.
> 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 u003C}} in the {{Encode.forJavaScript}} implementation. This will
> protect against both the malicious comment injection and the injection of
> closing script tags {{raw 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)
[jira] [Comment Edited] (SLING-8879) Make JSONObject#toString and XSSAPI#encodeForJSString both safe and correct for pasting into a javascript string literal
[
https://issues.apache.org/jira/browse/SLING-8879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16989033#comment-16989033
]
Ilguiz Latypov edited comment on SLING-8879 at 12/5/19 6:33 PM:
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML
or XHTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} against the mandatory HTML entity decoding in
XHTML (but not HTML) documents.
was (Author: ilatypov):
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML
or XHTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} against the mandatory HTML entity decoding in
XHTML (but not HTML) documents. This decoding applies to text both outside and
inside the script tag.
> 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 u003C}} in the {{Encode.forJavaScript}} implementation. This will
> protect against both the malicious comment injection and the injection of
> closing script tags {{raw 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)
[jira] [Comment Edited] (SLING-8879) Make JSONObject#toString and XSSAPI#encodeForJSString both safe and correct for pasting into a javascript string literal
[
https://issues.apache.org/jira/browse/SLING-8879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16989033#comment-16989033
]
Ilguiz Latypov edited comment on SLING-8879 at 12/5/19 5:58 PM:
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML
or XHTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} against the mandatory HTML entity decoding in
XHTML (but not HTML) documents. This decoding applies to text both outside and
inside the script tag.
was (Author: ilatypov):
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} against the mandatory HTML entity decoding in
XHTML (but not HTML) documents. This decoding applies to text both outside and
inside the script tag.
> 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 u003C}} in the {{Encode.forJavaScript}} implementation. This will
> protect against both the malicious comment injection and the injection of
> closing script tags {{raw 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)
[jira] [Comment Edited] (SLING-8879) Make JSONObject#toString and XSSAPI#encodeForJSString both safe and correct for pasting into a javascript string literal
[
https://issues.apache.org/jira/browse/SLING-8879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16989033#comment-16989033
]
Ilguiz Latypov edited comment on SLING-8879 at 12/5/19 5:58 PM:
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} against the mandatory HTML entity decoding in
XHTML (but not HTML) documents. This decoding applies to text both outside and
inside the script tag.
was (Author: ilatypov):
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} without a specific concern.
> 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 u003C}} in the {{Encode.forJavaScript}} implementation. This will
> protect against both the malicious comment injection and the injection of
> closing script tags {{raw 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)
[jira] [Comment Edited] (SLING-8879) Make JSONObject#toString and XSSAPI#encodeForJSString both safe and correct for pasting into a javascript string literal
[
https://issues.apache.org/jira/browse/SLING-8879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16989033#comment-16989033
]
Ilguiz Latypov edited comment on SLING-8879 at 12/5/19 5:55 PM:
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode with {{raw u}}
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} without a specific concern.
was (Author: ilatypov):
I also see a suggestion to fool-proof more characters serialized as JSON
strings in case these strings are embedded into a javascript embedded in HTML.
https://security.stackexchange.com/questions/11091/is-this-json-encoding-vulnerable-to-cdata-injection/11097#11097
This suggests to encode
* the opening angle bracket {{raw }} against closing the script tag or
opening a comment tag,
* the closing angle bracket {{raw }} against closing a possible CDATA
wrapper around the javascript text embedded in XHTML (and HTML?),
* {{U+2028}} and {{U+2029}} allowed in JSON but not in Javascript against
disrupting the javascript engine.
* the ampersand {{raw }} without a specific concern.
> 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 u003C}} in the {{Encode.forJavaScript}} implementation. This will
> protect against both the malicious comment injection and the injection of
> closing script tags {{raw 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)
[jira] [Comment Edited] (SLING-8879) Make JSONObject#toString and XSSAPI#encodeForJSString both safe and correct for pasting into a javascript string literal
[
https://issues.apache.org/jira/browse/SLING-8879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16989011#comment-16989011
]
Ilguiz Latypov edited comment on SLING-8879 at 12/5/19 5:40 PM:
I see SLING-6679 replaced JSONObject with Johnzon in year 2017 which has a
simple serializer that does not reuse the "for javascript" encoder. On the
other hand, Johnzon would not protect against the closing script tag injection
or the open comment tag injection in cases where the javascript string literal
is embedded with the HTML response.
I see Johnzon substitutes the double quotes, as required by JSON and by the
need to protect against the attack where malicious payload drops out of the
javascript string literal context in a Javascript embedded in HTML. Of course.
It would be nice if Johnzon replaced the open angle bracket {{raw }} with
an ASCII presentation of the respective Unicode, {{raw u003C}}, which is
also compatible with both JSON and Javascript.
https://github.com/apache/johnzon/blob/master/johnzon-core/src/main/java/org/apache/johnzon/core/Strings.java#L64
XSSAPI encodeForJSString() could reuse Johnzon's above serialization. (I guess
this would require accessing Johnzon via Sling's service provider interface to
an abstract JSON implementation).
Our older AEM code still uses the older JSONObject, but I guess there would not
be a security (built-in XSS protection against abuse of javascript code with
string literals embedded in HTML) or stability (strict javascript engine
failing to parse the backslash-forwardslash encoding I observed in
JSONObject#toString) update to that version of Sling.
was (Author: ilatypov):
I see SLING-6679 replaced JSONObject with Johnzon in year 2017 which has a
simple serializer that does not reuse the "for javascript" encoder. On the
other hand, Johnzon would not protect against the closing script tag injection
or the open comment tag injection in cases where the javascript string literal
is embedded with the HTML response.
I see Johnzon substitutes the double quotes, as required by JSON and by the
need to protect against the attack by dropping out of the javascript string
literal context in a Javascript embedded in HTML. Of course.
It would be nice if Johnzon replaced the open angle bracket {{raw }} with
an ASCII presentation of the respective Unicode, {{raw u003C}}, which is
also compatible with both JSON and Javascript.
https://github.com/apache/johnzon/blob/master/johnzon-core/src/main/java/org/apache/johnzon/core/Strings.java#L64
XSSAPI encodeForJSString() could reuse Johnzon's above serialization. (I guess
this would require accessing Johnzon via Sling's service provider interface to
an abstract JSON implementation).
Our older AEM code still uses the older JSONObject, but I guess there would not
be a security (built-in XSS protection against abuse of javascript code with
string literals embedded in HTML) or stability (string javascript engine
failing to parse the backslash-forwardslash encoding I observed in
JSONObject#toString) update to that version of Sling.
> 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 u003C}} in the {{Encode.forJavaScript}} implementation. This will
> protect against both the malicious comment injection and the injection of
> closing script tags {{raw script>}} forcing the javascript
> interpreter to drop out of the string literal context and drop out of the
> script context.
> The existing prefixing of
