Yes, you read that right. This site all about learning hacking with various posts about Cross-Site Scripting (XSS) vulnerabilities, had one such vulnerability itself! This goes to show that anyone can make mistakes, no matter their expertise. This post will detail what the cause of the vulnerability was and how it could not be exploited due to a strict Content Security Policy header.

Introduction

As you might have seen in the page footer, this website is fully Open source on GitHub. Everything is written in raw PHP, and you can read how everything works. With this comes also a bit of a risk, where potential attackers can plan and exactly test source code for vulnerabilities, without having to guess. But on the other hand, it also helps with source code review by random strangers. Luckily, this time it was the latter, as someone told me they found a Reflected XSS on my site and challenged me to find it as well. Soon enough I found it and confirmed with the person that they had found the same thing.

PHP...

There is a reason PHP gets a bad reputation. I would describe it as a very manual language, where you have to do most things yourself. There are a few basic built-in functions, but it's nothing compared to modern-day frameworks. Many security considerations you also have to implement yourself, like making sure to use prepared statements for SQL queries, or using encoded HTML entities to protect against XSS. Because of all these things you have to think of, you are very prone to making mistakes.

So why did I choose PHP? Mostly because it is what I started with, and am most comfortable with. It's a really easy language to learn at first as it is basically just extending HTML with a backend. I haven't found a good reason to spend time and rewrite the whole website in another framework, other than learning. The functionality will likely stay the same. It might happen if I have a lot of free time, but not now.

There are two sides to my site, the public one and the admin one. Only I (should) have the password for the admin side, and I know I won't attack myself (hopefully) so the admin side has a lot fewer protections needed. However, on the public side, there are a few places where user input can be provided, so these parts are where the most protections are. Think using htmlspecialchars() around echo'ed input to encode HTML entities, or using self-made my sql_query() function that uses prepared statements making SQL injections impossible. But unfortunately, as you will quickly find out, PHP sometimes does unexpected things, another reason to be careful with it.

Let's get to the point. The code snippet this is all about is the following:

$html_messages = array(
    "error_post" => '<div class="alert alert-danger animated bounceOut" role="alert">This post <b>does not exist</b></div>',
    ...
    "captcha" => '<div class="alert alert-danger animated bounceOut" role="alert">reCAPTCHA invalid</div>',
);

function displayMessage(): void
{
    global $html_messages;
    if (isset($_GET["message"])) {
        echo $html_messages[$_GET["message"]];
    }
}

This function is found in includes/all.php, a file that is included at the start of every single page. It contains many utility functions for repeated use. This displayMessage() function is used on all pages where you can be redirected after some message. For example, if you try to visit a non-existing post, it will redirect you to /blog/?message=error_post, which will then turn into a "This post does not exist" message when loaded. The function maps a short pre-defined message to a longer HTML string.

So what is wrong here? HTML messages come from a pre-defined array, and only those can be echo'ed back. The ?message= user input is not reflected by itself. Or is it?

This is where PHP comes in. What happens when we provide a value for ?message= that is not in the array? Sure it will simply give an error or something right? Well actually, it displays a warning in the response including the array key unescaped 🤦‍♂️.
Adding a parameter like ?message=anything will show an error like this:

Warning: Undefined array key "anything" in ...

And when you include HTML in that key, of course, it happily returns the warning with the HTML unescaped, meaning a payload like <img src=x onerror=alert()> instead of "anything" would be interpreted as HTML and would trigger the onerror= event, executing any javascript code included there resulting in XSS, where cookies could be stolen, or actions could be performed on my behalf.

Content Security Policy

Luckily, when you tried to trigger the vulnerability with a URL like the following:

https://jorianwoltjer.com/blog/?message=%3Cimg%20src%3Dx%20onerror%3Dalert()%3E

it would not actually pop an alert(), because the following error would be shown in the DevTools console:

Refused to execute inline event handler because it violates the following Content Security Policy directive: script-src 'self' 'nonce-WUAok...'.

I have some experience in security and know how annoying it sometimes is to find an XSS, but be blocked by the CSP from actually exploiting it. That's exactly why I added it to my own website as well. It is a really powerful mechanism to ensure that only scripts, styles, and other content can only come from your server.

The script-src directive defines where executed JavaScript code may come from. The browser will enforce this, by blocking the execution of any scripts that don't follow these rules.
The first 'self' refers to anything hosted on the same domain so in this case jorianwoltjer.com. I can <script src=...> any URL that is on that same domain, but it will refuse to load scripts from other domains.
The second 'nonce-WUAok...' refers to a really powerful trick in CSP that allows you to provide a randomly generated value unique for each request called the "nonce". You put this value in the CSP header as you see here, and it will require any inline scripts to have a nonce= attribute with that same random nonce value. Because this is unique for every request, you as the server can generate it, put it in the CSP, and put it in all the inline scripts. But then an attacker would not know the random value that will be generated beforehand, so they cannot predict what the nonce= attribute should be for their malicious script. This means the only inline scripts that may execute are ones generated by the server, as only they can know the nonce at the time of the request.

Tip: Content-Security-Policy is simply a header you can add to each response, and you can validate one for misconfigurations using the handy CSP Evaluator by Google

The Patch

Fixing this specific vulnerability was pretty simple. Simply checking beforehand if the key exists in the array will make sure the warning never gets displayed. See this commit for the actual fix:

function displayMessage(): void
{
    global $html_messages;
-   if (isset($_GET["message"])) {
+   if (isset($_GET["message"]) && array_key_exists($_GET["message"], $html_messages)) {
        echo $html_messages[$_GET["message"]];
    }
}

While this was pretty easy to fix with knowledge of the vulnerability, it is not easy to spot without much experience in PHP. I'm sure these kinds of issues exist in more places than my site, and I will look out for them in future code reviews.

Conclusion

I wanted to make this post to show that everyone makes mistakes. While we security researchers might laugh at a dumb vulnerability, developers aren't dumb. Mistakes can be very unexpected, especially if you don't have tons of experience with the technology you are using. Another lesson to learn is that Content Security Policy (CSP) is a really effective way of eliminating the impact of a Cross-Site Scripting (XSS) vulnerability, just make sure that it also cannot be bypassed.