Escape JSON to avoid parsing the content as HTML#232
Merged
RyanGlScott merged 1 commit intohaskell:masterfrom Nov 17, 2020
Merged
Escape JSON to avoid parsing the content as HTML#232RyanGlScott merged 1 commit intohaskell:masterfrom
RyanGlScott merged 1 commit intohaskell:masterfrom
Conversation
RyanGlScott
reviewed
Nov 16, 2020
Member
RyanGlScott
left a comment
RyanGlScott
left a comment
There was a problem hiding this comment.
Good catch! Is this bug present in all versions of criterion, or only after #229?
Contributor
Author
I've only tested with criterion-1.5.8.0 but I think it would be possible to inject HTML in previous versions as well. I'll test and see if it breaks in previous versions as well. EDIT:
|
considerate
force-pushed
the
master
branch
2 times, most recently
from
b6cf1e7 to
b7982c5
Compare
RyanGlScott
reviewed
Nov 16, 2020
By crafting a malicious string it was quite easy to escape the HTML attribute that contained the JSON report data. This commit aims to mitigate the breakage of the reports when report names contain unexpected characters. The JSON data is moved to a <script> tag and the content is escaped by replacing significant HTML characters with their corresponding JSON escape sequences. The '<' character is replaced with the sequence "\u003c", '&' is replaced with "\u0026" and so on.
RyanGlScott
approved these changes
Nov 17, 2020
Member
|
Thanks again! I've uploaded |
Contributor
Author
|
@RyanGlScott Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
By crafting a malicious string it was quite easy to escape the HTML
attribute that contained the JSON report data allowing injection of
arbitrary HTML using the report names.
This commit aims to mitigate the breakage of the reports when report names
contain unexpected characters. The JSON data is moved to a <script> tag and
the content is escaped by replacing significant HTML characters with their
corresponding JSON escape sequences. The '<' character is replaced with the
sequence "\u003c", '&' is replaced with "\u0026" and so on.
The processing of
/is still not ideal, if a report name contains that characterthen it is treated as a group separator.