Is this sanitizer vulnerable to XSS?

Question

To have a simple and secure editor for text fields of a django app, I have this snippet to sanitize input HTML into django code:

from bs4 import BeautifulSoup

def sanitize_html(value):
  tag_whitelist = ['img','b','strong','blockquote', 'a']
  attr_whitelist = ['src', 'alt', 'width', 'height', 'href','class']
  soup = BeautifulSoup(value)
  for tag in soup.find_all():
      if tag.name.lower() in tag_whitelist:
          tag.attrs = { name: value for name, value in tag.attrs.items() 
              if name.lower() in attr_whitelist }
      else:
          tag.unwrap()   

  # scripts can be executed from comments in some cases
  try:
    comments = soup.find_all(text=lambda text:isinstance(text, Comment))
    for comment in comments:
      comment.extract()
  except:
    pass
  return unicode(soup)

I have also blacklisted entering javascript in model fields using this method:

BADLIST = ['javascript']

def no_js (text):
    if any(e in text for e in BADLIST):
        raise ValidationError("Your text contains bad words!")
    else:
        return True

On the other hand, in template I need to use {{text| safe}} to allow healthy HTML tags to be displayed.

So I'm wondering with these constraint, whether the input is still vulnerable to cross-site scripting (XSS)? And if so, how to fix it?

Answer 1

The class attribute could be used for redressing the UI to make untrusted elements and text appear to be authoritative text coming from the website itself.

See the Google Browser Security Handbook for more information.

Instead of writing your own, use an established HTML sanitiser such as Google Caja. These are hard to write because there are so many ways of getting round XSS filters.

Also make sure you implement a Content Security Policy - then anything sneaking round Google Caja will refuse to execute in the browser.

Answer 2

BeatifulSoap is not designed as a Sanitizer for HTML but primarily designed for extracting data from the HTML like needed in screen scraping. That is don't expect it to deal correctly with malformed HTML which nevertheless gets executed by the browser.

Apart from that a large part of your question is already answered in your previous one. And to cite myself from an answer I wrote there:

If you really want to allow HTML treat it like any other kind of markup, i.e. parse it into an internal form and then create the resulting HTML out of this.

That's exactly what you don't. That is you consider all HTML sane where you don't find bad stuff instead of parsing it into an internal form and create a definitely sane and well formed HTML from it which only includes things you have explicitly white listed. For instance the user is able to set the class attribute to anything which is dangerous like I wrote in my answer to your previous question.