forked from chef/supermarket
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Kramer, Rudi
committed
Mar 11, 2019
1 parent
e4ee877
commit d11d633
Showing
1 changed file
with
7 additions
and
7 deletions.
There are no files selected for viewing
This file contains 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
d11d633
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<img src=\"//img.example.com\" alt=\"\">
but the gfm changes the html to<img src=\"//img.example.com\" alt=\"\" style=\"max-width:100%;\"></a></p>"
.Fixing the above issues was pretty simple and get us down to 5 failing tests.
javascript:alert(1)
which passes through the first filter, the MarkdownFilter, and is converted to"<p>[a](javascript:alert\xFE(1))</p>"
. The\xFE
is not valid utf-8 so it fails on the very next filter and the tests also fail with an invalid utf-8 error. I fired up a supermarket server on my local machine and the page fails to load with the same invalid UTF-8 error message.This accounts for 2 failing tests and I have no idea how to tackle this one. One train of thought is that if people are messing around with javascript encoding and their supermarket page doesn't display it's their own fault but it could reflect badly on Chef so I'm not sure...
MarkdownHelper to prevent XSS attacks does not render an HTML anchor for link text
Failure/Error: expect(helper.render_markdown("a")).not_to match(/<a href/)
expected "
<a href="javascrip&#...p;#x74('XSS')" target="_blank">a
" not to match /<a href/Diff:
@@ -1,2 +1,2 @@
-/<a href/
+"
<a href="javascript:alert('XSS')" target="_blank">a
"Even though it creates the link it doesn't function as a javascript function. When I tested it on my local server it created the following link
localhost:3000/cookbooks/javascript:alert('XSS')
which does't go anywhere or call any js.This makes me think that we need to change these two tests to match the behaviour of the markdown_renderer.
4.The last falling test is MarkdownHelper to prevent XSS attacks does not render an HTML img tag for funky alt text.
The tests expect the helper not to generate
<img src
but the helper does. I've tested against the demo app and even though it generates the src<img src="x" alt="alt text'"
onerror=prompt(document.cookie)" style="max-width:100%;">` The dodgy alt text is displayed and not run.