2
\$\begingroup\$

I have the following code which I have written due to the fact the Microsoft's Sanitizer is now to aggressive.

What I'm trying to do is as follows.

  1. Create a whitelist of HTML tags I want to keep
  2. After the tags have been converted, run the text through the sanitizer and remove any tags I do not require.
  3. After that, remove any tags that have either id's or classes inside them
  4. Finally convert the tags back.

My code is below, and it would be much appreciated if someone with more experience can have a look over it and give me some feedback.

string txt = "<h1>Keep as sanitizer removes</h1><p>keep</p><p class=\"remove\">remove class</p><br/><script>remove</script>";
            var whiteList = new List<Word>();
            whiteList.Add(new Word() { SearchWord = "<p>", ReplaceWord = "&lt;p&gt;" });
            whiteList.Add(new Word() { SearchWord = "</p>", ReplaceWord = "&lt;/p&gt;" });
            whiteList.Add(new Word() { SearchWord = "<h1>", ReplaceWord = "&lt;h1&gt;" });
            whiteList.Add(new Word() { SearchWord = "</h1>", ReplaceWord = "&lt;/h1&gt;" });
            whiteList.Add(new Word() { SearchWord = "<br/>", ReplaceWord = "&lt;br/&gt;" });

            whiteList.ForEach(w => txt = txt.Replace(w.SearchWord, w.ReplaceWord));

            var remove = Sanitizer.GetSafeHtmlFragment(txt);

            whiteList.ForEach(w => remove = remove.Replace(w.ReplaceWord, w.SearchWord));

            var again = RegexHelpers.StripHtmlAttributes(remove);

            var tt = again;



public static string StripHtmlAttributes(string s)
    {
        const string pattern = @"\s.+?=[""'].+?[""']";
        var result = Regex.Replace(s, pattern, string.Empty);
    return result;
}
\$\endgroup\$
3
\$\begingroup\$

You could simplify the declaration of the tags you want to keep, as it is very structured:

var whitelist = new List<string>(new string[] { "p", "h1", "br" });

foreach (var w in whitelist) {
  txt = txt.Replace("<" + w + ">", "&lt;" + w + "&gt;")
           .Replace("<" + w + "/>", "&lt;" + w + "/&gt;"));
}

var remove = Sanitizer.GetSafeHtmlFragment(txt);

foreach (var w in whitelist) {
  remove = remove.Replace("&lt;" + w + "&gt;", "<" + w + ">")
                 .Replace("&lt;" + w + "/&gt;", "<" + w + "/>"));
}

var again = RegexHelpers.StripHtmlAttributes(remove);

var tt = again;
\$\endgroup\$
3
\$\begingroup\$

Apparently, in general a regex isn't the right tool to parse HTML. I'm not sure why that is, but the fact that ...

<a href="foo" title="5>3"> Oops </a>

... is a valid tag might have something to do with it.

Therefore, to parse HTML, some people recommend you use a tool like Html Agility Pack.


Your code looks safe to me because of the way it's white-listing.

But some specific problems with it include:

  • You don't whitelist tags which have any attributes, e.g. <h1 id="foo">
  • Any &gt;h1&lt; in the original text will be converted to <h1>

To guard against the second problem, you might try to convert & to &amp; first, and then convert it back again.

\$\endgroup\$
  • \$\begingroup\$ Hi ChrisW thanks for reply, this section of code StripHtmlAttributes(string s) removes all elements from tags so if <h1 id="foo"> was entered, only <h1> would be returned Thanks \$\endgroup\$ – George Phillipson Feb 10 '14 at 14:42
  • \$\begingroup\$ It seems to me that you're not calling StripHtmlAttributes until after you call whiteList.ForEach(w => txt = txt.Replace ... and therefore your replace will fail to replace any whitelisted elements which contain an attribute. \$\endgroup\$ – ChrisW Feb 10 '14 at 14:47
  • \$\begingroup\$ Hi ChrisW thanks just tested again and I see what you mean, thanks \$\endgroup\$ – George Phillipson Feb 10 '14 at 16:57
  • \$\begingroup\$ Actually, parsing code like that using regular expressions is not a problem. The reason why HTML is not a regular language is nesting: you can't parse something like <i><b><i>oops</i></b></i> using regular expressions. \$\endgroup\$ – svick Feb 14 '14 at 20:17

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.