3

I don't know if I am doing this right.
I first time build something to prevent attack on page.
I will start from the bottom:
I have property:

public string Description {get;set;}

User can set it's value through tinyMCE

tinyMCE.init({
            mode: "textareas",
            theme: "advanced",
            encoding : "xml"...

In controller before I save this in database I do:

model.Description = HttpUtility.HtmlDecode(model.Description);

In database I have a value like:

<p>bla bla bla</p>

I added AntiXSS library to my project:

public class AntiXssEncoder : HttpEncoder
    {
        public AntiXssEncoder() { }

        protected override void HtmlEncode(string value, TextWriter output)
        {
            output.Write(Encoder.HtmlEncode(value)); // on breakpoint code always get in here
        }
...

When I display data from database I use:

@Html.Raw(Model.Place.Description)

And it works fine I see only text. No Html tags. Breaklines work fine. I can style text with bold, italic etc.

But If I enter:

<script>alert(open to attack);</script>


I got alert window.
I don't understand do I need to do something more to prevent this?

1110
  • 7,829
  • 55
  • 176
  • 334
  • 1
    Wait, that code makes no sense. You're encoding, then deencoding, then outputting as raw HTML? If you want to disallow certain tags you should simply strip them out server side. Force TinyMCE to XHTML mode and use an XML parser server-side to remove what you want. – Rudi Visser Jan 30 '13 at 20:37
  • I am completely lost. I don't have idea what to fix anymore. All I want is to allow user to enter bold text and lists but not script tags to break page but what ever I do it's wrong... – 1110 Jan 30 '13 at 21:09
  • 1
    Get rid of that `AntiXssEncoder` class that's doing, well, nothing useful (if anything at all since it makes no sense), and just replace out the tags you don't want in there. If you don't want to force TinyMCE into XHTML mode, you could do something really crude like checking for ` – Rudi Visser Jan 30 '13 at 21:22
  • @RudiVisser unfortunately that won't stop dangling Html attacks – Adam Tuliper Jan 30 '13 at 21:28
  • @AdamTuliper Hence the "crude" comment :) – Rudi Visser Jan 30 '13 at 22:02

2 Answers2

5

I added AntiXSS library to my project

And where are you using it?

Make sure that you have not only added AntiXSS but you actually used it:

@Html.Raw(Microsoft.Security.Application.Sanitizer.GetSafeHtmlFragment(Model.Place.Description))

But remember that the new version of the AntiXSS library is a bit too restrictive and will strip tags like <strong> and <br> out which might not be desired.

As an alternative to the AntiXSS library you could use HTML Agility Pack to do this job. Rick Strahl blogged about a sample implementation.

carla
  • 1,970
  • 1
  • 31
  • 44
Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
  • I have removed all `HttpUtility.HtmlDecode` and added your code but now I get `potentially dangerous Request.Form` error. I use .NET 4. – 1110 Jan 30 '13 at 21:44
  • Make sure your `Description` property is decorated with the `[AllowHtml]` attribute, just like that: `[AllowHtml] public string Description { get; set; }` – Darin Dimitrov Jan 30 '13 at 21:45
  • Now in db I have `

    test test

    ` and on view I have `

    test test

    `
    – 1110 Jan 30 '13 at 21:59
  • Great, isn't it exactly what you wanted? – Darin Dimitrov Jan 30 '13 at 22:01
  • 1
    @1110 .. that's exactly what you want. – Rudi Visser Jan 30 '13 at 22:01
  • No :) I don't want that user see tags. Just bolded text. Now I added encoder while saving and in db is `<p>test <strong>test</strong></p>` but the same is on page :( – 1110 Jan 30 '13 at 22:03
  • But get rid of all html encodings and everything. Store the text in the database exactly as it is entered by the user. Only when outputting on the view use `Html.Raw` along with the `Encoder.HtmlEncode` method as shown in my answer. In your database you should store `

    test test

    and on view I have

    test test

    ` without any `<` and html entities.
    – Darin Dimitrov Jan 30 '13 at 22:06
  • I copy paste your code and this is what I get on view `<p>test <strong>test</strong></p>` – 1110 Jan 30 '13 at 22:08
  • And what do you have in your database? – Darin Dimitrov Jan 30 '13 at 22:10
  • Exactly this `<p>test <strong>test</strong></p>` – 1110 Jan 30 '13 at 22:10
  • Well that's your problem. Didn't you read my comments? In your database you should store unencoded text, like this: `

    test test

    and on view I have

    test test

    `. Why did you HTML encode the text before storing it into the database?
    – Darin Dimitrov Jan 30 '13 at 22:12
  • Ok now I removed all Html.Decode from controller. So I write `test test` in tinyMCE and set second test word to bold and save to database. In database I have `

    test test

    ` on the page where I need to display this as a normal text (without tags just test and bolded test) I get this `

    test test

    ` (with tags) code to display this is: `@Html.Raw(Microsoft.Security.Application.Encoder.HtmlEncode(Model.Place.Description))`
    – 1110 Jan 30 '13 at 22:16
  • Sorry I made a mistake in my answer. You should use `Sanitizer.GetSafeHtmlFragment` instead of `Encoder.HtmlEncode`. I have updated my answer to reflect this. – Darin Dimitrov Jan 30 '13 at 22:21
  • But don't forget that the AntiXSS library will strip the `` tags out. – Darin Dimitrov Jan 30 '13 at 22:21
  • So this saves me from script attack. But I can't have bold text saved and displayed. Am I right when I say that I can't have both? – 1110 Jan 30 '13 at 22:25
  • Yes, you are right in saying that. The AntiXSS library is a bit too restrictive. You could use the HTML Agility Pack and build your own sanitizer. Checkout Rick Strahl's blog post on this topic: http://www.west-wind.com/weblog/posts/2012/Jul/19/NET-HTML-Sanitation-for-rich-HTML-Input – Darin Dimitrov Jan 30 '13 at 22:27
  • Indeed, use GetSafeHtmlFragment as I mentioned below before saving. The approach above still allows script to get into the database, potentially displayed in other areas as well. Sanitize before saving (nothing wrong with double sanitizing though - once on save and once on display) – Adam Tuliper Feb 01 '13 at 22:50
1

First off you are displaying RAW Html- not encoded in any manner. If you want to display Html you should ideally be doing several things.

  1. Sanitize it with the antixss Sanitizer class using GetSafeHtmlFragment. Note that this wont protect you completely. Do this before saving to the DB.

  2. Implement the not yet fully supported headers to prevent other script from running. This is limited to only some of the modern browsers.

  3. Or... Dont allow html or don't allow any HTML outside of known character tags. Ie a whitelist approach so you allow <strong> and nothing outside of other alphanumeric chars is allowed.

Rudi Visser
  • 21,350
  • 5
  • 71
  • 97
Adam Tuliper
  • 29,982
  • 4
  • 53
  • 71
  • And is there some example on how to allow only tags like: ,
    • ?
    – 1110 Jan 30 '13 at 21:14
  • Ah I forgot toStaticHtml as well – Adam Tuliper Jan 30 '13 at 21:19
  • There is no simple answer here. You are allowing a dangerous operation so it's up to you now you want to mitigate this. If you only want strong, you'll have to parse out any < > that don't have strong in between them, if that's all you want to support. If you want to support more tags, it gets more difficult. There are many gotchas here too- are you supporting ie6? Many big bugs there from a security standpoint. There's no easy answer except if you want limits tag support to parse for your tags and replace any other script chars. – Adam Tuliper Jan 30 '13 at 21:27