Security Improvement: Added Content-Security-Policy

[Plazmaz]

Also....

Fixed cross-site scripting bugs in Daily Hits chart and stream view.

[Plazmaz]

Hi all, just pinging you to check in. Despite the inconspicuous title, this is actually a fairly sensitive set of security fixes worth merging. Apologies for the holiday ping, but it's been over a month since I made this PR and right now it'd be pretty easy to exploit this on your live server.

@gjbae1212 @jungdu @jdevstatic @ryanfortner

[jungdu]

Content-Security-Policy (CSP) header has been applied as a header in the SVG response.

You can apply a Content-Security-Policy (CSP) using one of the following methods:

1. Set Content-Security-Policy in the HTTP headers of the page response.
2. Add it in the tag in the HTML.

Please let me know if any of the information I provided was incorrect

[Plazmaz]

Content-Security-Policy (CSP) header has been applied as a header in the SVG response.

You can apply a Content-Security-Policy (CSP) using one of the following methods:

1. Set Content-Security-Policy in the HTTP headers of the page response.

2. Add it in the  tag in the HTML.

Please let me know if any of the information I provided was incorrect

That's all correct. This PR also fixes two exploitable cross-site scripting bugs that would permit an attacker to run code in the browsers of anyone who visits your site.

[jungdu]

It seems that the Content-Security-Policy (CSP) you added is not for the response of the website, but for a chart image response. The Content-Type of the response is svg.

[Plazmaz]

Yep, in this case I explicitly restricted the CSP for the chart SVG because it has no need to include external scripts. This provides an additional layer of mitigation (SVG files can include HTML and other misc things)

[jungdu]

Thank you for your good comment.
Could you tell what potential damages may occur if Content-Security-Policy is not implemented in this project?
I'm curious because this project does not handle any customer information or login functionality.

[Plazmaz]

Hi, to be clear that isn't actually the main point of this PR. the main point is the switch from innerHTML to textContent, preventing DOM XSS, and calling html.EscapeString on the title in the daily hits screen. These prevent trivial cross-site scripting exploits. the CSP was just additional and some cover to not drop the details of the exploit so publicly.

To trigger the vulnerability:

1. Open a tab with https://hits.seeyoufarm.com/ and open your browser's console
2. Open another tab and visit: https://hits.seeyoufarm.com/api/count/incr/badge.svg?url=https://github.com/gjbae1212/%3C/text%3E%3Cimg%20src=x%20onerror=%22console.log(%27xss%27)%22%20style=%22display:%20none%22/%3E%3Ctext%3E
3. Note that anyone viewing the homepage will see "xss" printed into their developer console.

What's happening here? Well, the URL decoded value of "url" is:

https://github.com/gjbae1212/</text><img src=x onerror="console.log('xss')" style="display: none"/><text>

The previous code (what's running right now) calls

p.Set("innerHTML", args[0].Get("data"))

This makes the HTML of the created "p" element:

<text></text><img src=x onerror="console.log('xss')" style="display: none"/><text>

which causes an image to be loaded from https://hits.seeyoufarm.com/x, which fails to load and triggers an error, which triggers the onerror handler of the img tag the attacker created, calling console.log. The fix included in this PR uses innerText instead of innerHTML and fixes a similar XSS issue in the daily hits page, which I can provide a proof-of-concept for as well if needed.

[jungdu]

Thanks for the detailed explanation. 👍