38

I am creating a website that allows people to upload HTML content.

Currently these are the tags that are banned:

<script></script>
<iframe>
<object>
<embed>
<style></style>
All on= attributes, i'm not gonna list them all there are like 70 of them

I don't want the user using any JavaScript or placing any code that will effect other tags on the page, so no style tags either.

Are there any other tags that I need to be mindful of when enabling users to generate HTML without impacting other things on the page?

  • 28
    There are libraries that can sanitize html for you. Use a server side one but which one depends on what framework you are using. There is no reason to have to reinvent the wheel and even if you really want to you could look at their implementation(s) and see what they do. – Igor Oct 30 at 14:59
  • 20
    Use a library. You'd be surprised how many ways there are around tag sanitizing... In general, I'd recommend against it entirely if possible, but if you go that route then a library with enough testing is your best bet. – Jarrod Christman Oct 30 at 15:01
  • 77
    Don't sanitize by banning tags! Rather, sanitize by allowing only a specific set of tags. cheatsheetseries.owasp.org/cheatsheets/… – Ghedipunk Oct 30 at 17:14
  • 5
  • 19
    How about <a href="javascript:alert('pwned');">Click for dancing bunnies</a>? – user253751 Oct 31 at 10:16
86

User-defined HTML

You're attempting to sanitize user input by blacklisting things you don't want to allow. Unfortunately, especially given the very large list of options for HTML5, it's quite easy to miss something. Missing something will result in a potentially dangerous XSS vulnerability, which you really don't want. To pick some random examples off the top of my head:

  1. Do you know that SVG tags can execute scripts in some (limited) circumstances?
  2. In your list of on event attributes you banned, did you make sure and get the onbounce event attribute of the obsolete (but still available) marquee tag?
  3. Are you going to make sure and keep track of any changes to the HTML spec that might roll out over the next X years, just in case one might add a dangerous tag/event?

There can be so many gotchas that securing input with a blacklist is nearly impossible. Moreover, you're approaching the problem from the wrong perspective. In security the approach you want to start with is the Principle of Least Privilege. Rather than asking, "what should I stop my users from doing", it's much safer to ask, "What should I allow them to do?". Therefore you need a two step process:

1. Robust Parser. You need a very robust parser. This is surprisingly difficult because browsers are so very forgiving when it comes to parsing HTML. If an attacker gives you bad HTML and your parser stops trying and says, "There's no HTML here - you're safe!", but the browser takes the same input, takes some guesses at what the HTML was supposed to be, and ends up with something malicious, you have an XSS vulnerability. This is even more difficult that it sounds because different browsers can apply different "corrections" when processing HTML input, making the already difficult task of parsing HTML reliably even more difficult.

As an example of browsers playing fast an loose with HTML, you can save this HTML to a file and load it in your browser:

<table><img src="1" <table onerror="alert(1)"
<p>hi</p></table>

If you load it up with Chrome and inspect element on the page, you'll see that the browser actually rendered this (probably depends on your version though):

<img src="1" <table="" onerror="alert(1)" <p="">hi<p></p><table></table>

That's an image tag with an active XSS payload (which is mildly obfuscated by the fact that a table tag and the p turned into meaningless attributes), a literal string hi, an empty p tag, and an empty table tag. The end result is quite different than the input. I haven't tried very hard to hide the payload, but would your parser have understood it the same way? Perhaps your parser would have tried to ignore the img tag inside of the table tag since anything not in a td tag would technically be illegal. Maybe your parser would have been confused by the <table inside the img tag and ignored the onerror since a table tag doesn't technically have events. As it turns out though, none of that stopped the browser from executing my javascript payload. Would your parser have caught it?

2. Whitelist allowed tags and attributes Once you've parsed the user's HTML, you don't want to compare with a blacklist and remove disallowed tags/attributes. Instead you want to compare to a whitelist and remove anything that you haven't specifically vetted and approved as safe. This puts your security on much stronger footing and - let's be honest - do you really want your users to use the marquee tag anyway?

Most importantly though, building a robust HTML parser is surprisingly difficult. If you try to do it yourself you'll spend a lot of time and probably get a lot wrong. Under normal circumstances, you'll be much better off just finding a well supported third party library to use.

Alternate Suggestion

A different tactic I would normally recommend would be to not let user's use HTML at all. Rather, allow a more limited language (like the markdown used for writing questions and answers in stack overflow). The more limited language specification makes writing a parser much easier and less error-prone, and the process of converting markdown to HTML makes it easier to guarantee "safeness" (note: easier, not guaranteed - markdown to HTML converters still suffer from occasional XSS vulnerabilities). There is a slight disadvantage that it limits the sort of formatting options users have (although I don't really consider that a disadvantage under most circumstances), but you also have a nice advantage that markdown parsers and HTML-converters are available in a wide variety of languages. A more limited option like this is typically a good trade off between usability and security. You can even add in a WYSIWYG editor that builds the markdown for users.

  • 10
    Although Stack allows both markdown and restricted HTML (plus MathJax on some) and silently removes other HTML (except in 'code'). This results in maybe 1 in 20 Qs in my experience becoming nonsensical or wrong because the poster tried to use angle brackets for variables (formally, nonterminals), and they disappear from the post. PS: 'tack' (direction or path, as of a boat) not 'tact' (consideration of people's sensitivities or propriety/custom). – dave_thompson_085 Oct 31 at 3:57
  • @dave_thompson_085 Interesting. I've never tried anything tricky here so haven't given too much thought to what they do behind the scenes. I would have expected that they first properly encoded special characters on output and then converted their markdown to HTML, causing HTML tags in input to come through as plain text. Obviously though they are doing something much more involved. I could see why it might be tricky to do things right, so perhaps they ended up going with a more layered approach (although one that obviously causes UI problems sometimes). – Conor Mancone Oct 31 at 12:28
  • You mentioned under 1 that browsers correct incorrect html. Can you give an example there? (not sure how extensive these corrections can be from the statement alone and....it surprised me a lot there) – Thomas Oct 31 at 13:22
  • 1
    @Thomas If browsers didn't have methods for dealing with incorrect HTML then they wouldn't be able to display most pages on the internet :) I'm a bit hesitant to add something to the answer as I think it's more of an aside than critical to the main point. However, consider the following HTML: <table><table><table> <p>hi</p> If I save that to an HTML file, open it up in chrome, and inspect element, Chrome has rendered this: <table></table><table></table><p>hi</p><table></table>. Using such changes to find an exploit is difficult, but plausible. – Conor Mancone Oct 31 at 13:38
  • 3
    Note that google has a similar problem in their site-preview, where they embed third-party content. Their solution to the parsing problem is to parse the html themselves, then filter that via whitelisting, then pretty-print it into conformant html that allows no room for browser interpretations. – WorldSEnder Nov 1 at 6:16
21

Currently these are the tags that are banned:

In addition to what's already been posted, make sure that banning means "don't letting the user save the data", not "remove the banned stuff and save the rest".

An example:

Input:
Hi there, here is my <script>alert('scary script')</script>, will I be shown?  
Filtered:  
Hi there, here is my alert('scary script'), will I be shown?

Seems good by just removing banned tags? After all, we stopped the script tag doing something stupid. But what if I instead enter this?

Input:  
Hi there, here is my <scr<script>ipt>alert('scary script')</scr<script>ipt>, will I be shown?  
Filtered:  
Hi there, here is my <script>alert('scary script')</script>, will I be shown? 

Whoops! Removing the <script> tags made my string a valid attack.

I used this trick to make a cooler looking profile page on a long forgotten social network that allowed you to add some custom HTML. Filtering HTML is hard, try to find some other way to achieve what you're after.

  • 3
    Welcome to security.stackexchange, and great first post! Trying to strip banned (or all) attributes is a common way for people to try to remove dangerous HTML, and it's not too crazy of an idea. Some languages even support it by default (we use strip_tags in our own systems). The trick (which strip_tags follows) is to keep looping through your tag removal algorithm until nothing is left. When trying to do it yourself though, it's a very easy thing to miss, and very bad when you do! – Conor Mancone Nov 1 at 13:06
  • aka, this is definitely one of those "gotchas" that people easily miss when they try to implement something like this without the experience needed to get it right. – Conor Mancone Nov 1 at 13:08
6

Leaving aside all the ways I could still slip XSS past this blacklist (which other answers have largely covered), allowing arbitrary HTML is still very dangerous. For example, if the user has control over the style= attribute (you said you blocked style tags, but nothing about inline attributes), or even just has access to the legacy positioning attributes (if you're letting users supply the HTML, the browser will have to allow somewhat-invalid input), the attacker can basically draw over the entire page with malicious content, such as a phishing login form, a (fake) security warning/ransomware extortion message, horrible images or videos, etc. Malicious users could also find HTML that adversely impacts a browser's rendering engine (such as consuming a ton of RAM and/or taking forever to render and using all their CPU; not all attacks are aiming to take control of a system) and spam that HTML everywhere they can.

The only good option is to use a safe layout language that gets translated into HTML by a well-tested library (the various forms of markdown or bbcode are intended for this). If you must allow HTML, do it by whitelisting specific tags, and within those tags whitelisting specific attributes (and if necessary only allowing specific values of those attributes), and throwing out anything that doesn't match. Then, re-testing after each change your filter makes to ensure the filter's own change didn't introduce a malicious input.

5

There are major risks with user generated content that is to be interpreted/parsed and displayed publicly. XSS attacks and the like can occur from users being able to sneak tags through your sanitizing functions, and there are a lot of variations that a browser will interpret that you must design for.

Recommendation? Do not allow tags at all. If you must, there are libraries out there (server side) that attempt this and are likely to have a lot of the work done to avoid the sanitizing bypasses that may exist.

In terms of your question:

Are there any other tags that i need to be mindfull of when enabling users to generate html without effect other things on the page?

The best approach is to assume all tags are of concern, and more so decide on the specific tags you want (as user Ghedipunk suggested). This is because tags can be exploited in unique, and sometimes unforeseen ways. This can be from specific browser implementation quirks to less common uses of tags. It's far easier to remove all input that follows the pattern of an HTML tag, with an exception for specific tags, than it is to try and prevent specific tags.

There are various ways to achieve this, from the most naive to far more advanced methods (actual DOM processing). If you try to do a blacklist approach, you'll be constantly chasing variations of different bypass attacks as well as unexpected uses of various tags you thought safe or forgot to include.

I'm confident a library already exist out there that has such operations done in a more advanced and secure fashion. My recommendation would be to seek those out for whatever platform you're developing for.

  • 2
    While "Do not allow tags at all" is likely good advice in general, the OP is specifically trying to allow users to provide HTML (likely for formatting reasons). As a result, you're basically telling the OP not to do what they are asking how to do. Sometimes a frame challenge is necessary, but there are suggestions that can legitimately be given here. – Conor Mancone Oct 30 at 17:29
  • @ConorMancone, I agree, but I am more so answering the question in the title of, "are there any security risks of using user generated html tags?," to which the answer is a large, yes. I'll edit my answer to clarify a bit more though. – Jarrod Christman Oct 30 at 17:35
  • 2
    There is no such thing as a naive approach to HTML parsing. Take for instance the user input: <img src="1" onerror="alert(1)" <br>. Step one results in <img src="1" onerror="alert(1)"{{break}}. Step 2 sees no html and makes no changes (since it may ignore the incomplete img tag). Then step three turns the result back into <img src="1" onerror="alert(1)" <br> which a browser will happily treat as an image tag, ignoring the <br, and resulting in an XSS vulnerability. – Conor Mancone Oct 30 at 18:45
  • 2
    Sure... except that your example of a naive approach likely leaves the system vulnerable to an XSS vulnerability. This is a security site - if you're going to give example code, then it really shouldn't have any security vulnerabilities. Someone somewhere is going to see your example, copy the general principles, and end up with a vulnerable site. Your answer doesn't need an example, so if you can't make one without a vulnerability (which is likely impossible due to the complexities involved) then you are better off just removing it. – Conor Mancone Oct 30 at 19:38
  • 1
    One would hope that information would be read in context and understood before attempting an implementation... but fair enough. I removed the pseudo code. – Jarrod Christman Oct 30 at 19:41
1

If it doesn't cause problems for your page layout, a good option would be to place the user-generated code inside an <iframe> and serve it from a separate subdomain or domain.

<iframe> is designed to isolate the content inside the frame from the page outside it. By serving the frame contents from a separate domain, you can also isolate it from any cookies the main site is using. Cookies from main domain (example.com) are also visible in subdomains (foo.example.com), but if the main site is in different subdomain (www.example.com) or completely different domain (example2.com), the sites would have different cookies.

The downside is that it's difficult to make the frame contents seamlessly merge with surrounding page, and it may cause e.g. separate scrolling bar for the frame to appear.

  • Can't a subdomain x.example.com get/set cookies from the domain example.com? So JS running in the subpage would be able to interfere in the page? – curiousguy Nov 2 at 23:25
  • @curiousguy Ah, true - I'll update the answer. – jpa Nov 3 at 6:30
0

This could lead to an attack vector which is called Cross Site Scripting (XSS) more on this here. You should not rely/trust on blacklisting the list of tags as there are many ways to bypass it in ever evolving HTML language.

Thumb rule is- use sandbox environment, sanitise the input and encode the output. There is various solution and frameworks at both ends (client and server side) that can perform such tasks for you like DOMPurify.

  • If the OP is trying to allow users to input HTML then encoding the output won't help, unfortunately... – Conor Mancone Oct 30 at 17:34
-3

Also in addition to all the dangers of parsing HTML, make sure that if you are saving this input in a database you watch out for SQL injection vulnerabilities.

One thing to start with is to use SQL parameters.

What is SQL injection?

  • Saving to a database does not equal SQLi. – schroeder Nov 1 at 18:40
  • 1
    While validation and sanitization of input before putting it into a database (and after getting it back out) is an important consideration, that isn't the topic of this question. – Ghedipunk Nov 1 at 19:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.