3

HTML comments can be notoriously tricky from an XSS standpoint, and the standard recommendation seems to be to never put untrusted user data in them. But shouldn't it be possible to strip out a few characters and make it safe?

My theory is that the following should be enough:

If I strip them out, am I safe? Implemented in PHP you get:

function echo_html_comment($content) {
    // Warning! This code is untested and therefore unsafe. Do not use!
    $content = str_replace(['<', '>', '[', ']', '`'], '', $content);
    echo "<!-- $content -->";
}

I consider character encoding exploits outside the scope of the question. You can also assume that comments will only be put between HTML tags, not inside attributes or in other strange places.

You could easily make the argument that the risk is not worth it for something as pointless as a HTML comment, and I might agree. But what I am interested in here is if the above filter is actually breakable?

1
  • in my opinion you should replace even the equal sign an the parentheis . tw if the user can manipulate an input in a script block a '-alert()-' is enogh. Commented Dec 28, 2021 at 17:38

1 Answer 1

1

There is a lot to unpack here, and so I will do my best you give you a fair and accurate response while keeping this relatively brief. Firstly, I don't think that your echo_html_comment() XSS sanitizer is effective but, rather, I think that you have defined it in such a way as to have an effective sanitizer. When you say that "I consider character encoding exploits outside the scope of the question", I think you are misunderstanding how XSS prevention works (no offense meant!). If you narrowly consider dumping user input into <!-- --> without any further context and stipulating oddly specific prohibitions (such as no character encodings, etc) then your XSS sanitizer seems legit. However, you gotta ask yourself not just what can happen directly when their input is put into a comment but also where is it being put in the DOM and how is it being dynamically interacted with (if at all). These are incredibly relevant questions that are broader than just analyzing if the attacker can insert XSS that will immediately execute. Let me try to explain with an example. Imagine that a developer codes in a div tag that puts uses your echo_html_comment() PHP function to dump the user's content into a comment in that div:

<div id="firstDiv">
    <span>Content 1</span>
    <span>Content 2</span>
    <?= echo_html_comment($userInput) ?>
</div>

Now, if we just look at the situation within this narrow scope (of what I've told you so far), it seems as though nothing bad can happen as the attacker has no viable means of breaking out of the html comment tag (as we are stipulating that the comment isn't placed in weird places, etc). However, let's ask ourselves how is the developer interacting with that div?: now we need to zoom out and analyze a broader scope. For example, let's say this is the code:

<div id="firstDiv">
    <span>Content 1</span>
    <span>Content 2</span>
    <?= echo_html_comment($userInput) ?>
</div>

<div id="secondDiv">

</div>

<script>
    const nodes = document.getElementById('firstDiv').childNodes;

    for (let i = 0; i < nodes.length; i++){
        document.getElementById('secondDiv').innerHTML += nodes[i].textContent;
    }

    document.getElementById('firstDiv').innerHTML = document.getElementById('secondDiv').innerText;
</script>

Now we have XSS! Why? Because the developer is manipulating that div, which encompasses the HTML comment they sanitized, in a way that introduces XSS--regardless of whether the developer realizes it or not (in this case, it is piss poor JS code). For example, if the $userInput were to be &lt;audio src=x onerror=alert(1)&gt;, then the XSS payload will execute just fine. I think this is the more fundamental thing that I think you are misunderstanding: it is misleading to assume that your echo_html_comment() function is actually safe where a developer were to be able to trust that they can put it anywhere that they normally would (aka, nested within an element) because XSS prevention is about determining not only what an attacker can influence directly but also how and where the developer is using it. I will admit that my example is a bit technical, but my point is that when someone says their sanitizer is XSS safe within a context, it should be safe no matter the how or the where (where within context, so I wouldn't hold it against you that they could execute XSS directly with quotation marks in an input value because the context you provided was that the HTML comment was used in between elements--which is how they are normally used). This leads me to my second point: always whitelist. Never, ever, ever, blacklist. As you kind of alluded to at the end of your post: is it really worth it? If someone uses your echo_html_comment, they are not guarenteed anything so long as you are blacklisting: attackers come up with incredibly complex and specifically tailored to the situation exploits (which includes char encoding). Attackers don't just look at what they can do directly (or immediately): they may plan a perfect exploit that doesn't actually trigger their XSS payload until their user input has gone down and completely through allison's rabbit hole. The rule of thumb is always only allow what you want them to be able to input. Then not only do you not have to try to think of all the evolving and incredibly complicated ways people come up with XSS but you also know exactly what to expect from them--which gives you the upper hand (advantage).

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .