18

I use the Newtonsoft library to convert C# objects into JSON. Is this use of Newtonsoft.Json.JsonConvert.SerializeObject secure, or is additional encoding necessary? If additional encoding is needed, what do you suggest?

Here is how I use it in a Razor view:

<script type="text/javascript">
    var jsModel = @Html.Raw(Newtonsoft.Json.JsonConvert.SerializeObject(Model))
</script>
Jeremy Cook
  • 20,840
  • 9
  • 71
  • 77

6 Answers6

14

You will at the very least need to perform additional encoding of the '<' character to '\u003C' and the '>' character to '\u003E'. Last I checked JSON.NET did not encode these characters in string literals.

I'm probably going to get flak for this, but the way I would do this is to render a dummy element onto the page:

<div id="the-div" data-json="@JsonConvert.SerializeObject(Model)" />

Then, in Javascript, extract the data-json attribute value from the the-div element and JSON.parse it. The benefit to this is that you don't need to worry about which characters require special encoding. The SerializeObject method guarantees that the JSON blob is well-formed, and the @ operator guarantees that any remaining non-HTML-safe characters left over from the JSON conversion are properly escaped before being put into the HTML attribute (as long as the attribute value is surrounded by double quotes, as above). So yes, it's a little uglier, but it is effective at completely shutting down an entire class of vulnerabilities.

Levi
  • 32,628
  • 3
  • 87
  • 88
  • Shouldn't `@Html.AttributeEncode(JsonConvert.SerializeObject(Model))` be used? – Jeremy Cook Mar 31 '14 at 15:40
  • '@Html.AttributeEncode' will double-encode output. The '@' operator by itself is all that is necessary, as it was designed to produce output which is safe both for regular HTML and for an HTML attribute. – Levi Mar 31 '14 at 16:01
  • `@` (e.g. `Html.Encode`) alone does seem acceptable in this context but so does `Html.AttributeEncode`. I don't think it will double-encode output but of course I'd like to know if I'm wrong. See http://stackoverflow.com/questions/2244079/what-is-the-difference-between-html-attributeencode-vs-html-encode/2244096#2244096 – Jeremy Cook Mar 31 '14 at 17:07
  • `@input` gets converted to `Response.Write(HtmlEncode(input))`. Thus `@Html.AttributeEncode(input)` will get converted to `Response.Write(HtmlEncode(HtmlAttributeEncode(input)))`, which results in double-encoding. We explicitly designed `@` to be safe both for regular HTML and for HTML attributes precisely so that developers didn't have to worry about the difference between HtmlEncode and HtmlAttributeEncode. – Levi Mar 31 '14 at 18:09
  • Ah you're right, I assumed `HtmlAttributeEncode` returned a `MvcHtmlString` but it returns a `string`. So, is it ever necessary to use `HtmlAttributeEncode`? – Jeremy Cook Mar 31 '14 at 18:39
  • Should never be necessary to use HtmlAttributeEncode going forward, as HtmlEncode covers all of HtmlAttributeEncode's use cases. (We're considering obsoleting HtmlAttributeEncode in vNext.) – Levi Mar 31 '14 at 18:46
  • do you have any comments on the answer I proposed? – Jeremy Cook Mar 31 '14 at 18:50
12

Using @Html.Raw alone like the question does is definitely dangerous. Here is another way to safely output a model within <script></script> tags. I followed @Levi's example to depend on the browser's faculties, as well as Microsoft's security features, and came up with this:

var jsModel = JSON.parse("@Html.Raw(HttpUtility.JavaScriptStringEncode(
    JsonConvert.SerializeObject(Model)
))");

I used the following very simple test. If I were only using @Html.Raw like in the question the "Bad" alert appears. Wrapped up in this way, I have valid JavaScript and the alert does not appear.

var jsModel = JSON.parse("@Html.Raw(HttpUtility.JavaScriptStringEncode(
    JsonConvert.SerializeObject(new {
        Test = "</script><script>var test = alert('Bad')</script>"
    })
))");

The next step would be to wrap this up in a reusable HtmlHelper extension method.

Jeremy Cook
  • 20,840
  • 9
  • 71
  • 77
3

I made this JsonConverter that encodes all strings with Microsoft Web Protection Library-library (aka AntiXSS-library) (http://wpl.codeplex.com/):

/// <summary>
/// To be used when you're going to output the json data within a script-element on a web page.
/// </summary>
public class JsonJavaScriptEncodeConverter : Newtonsoft.Json.JsonConverter
{
    public override bool CanConvert(Type objectType)
    {
        return objectType == typeof(string);
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        return reader.Value;
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        writer.WriteRawValue(Microsoft.Security.Application.Encoder.JavaScriptEncode((string)value, true));
    }
}

Usage:

<script type="text/javascript">
    var jsModel = @Html.Raw(Newtonsoft.Json.JsonConvert.SerializeObject(Model, new JsonJavaScriptEncodeConverter()))
</script>
Torbjörn Hansson
  • 18,354
  • 5
  • 33
  • 42
0

I don't think it's necessarily unsafe here but it depends on the data. If you data has been sanitized, which it always should if it came from an outside source, then you are probably fine. The fact that it's going into a javascript object and not rendered as HTML obscures things a bit but it still comes down to your level of trust with the data being output.

Brad Gardner
  • 1,627
  • 14
  • 14
  • The data is not from a trusted source. Another way to look at it, even if it were from a trusted source I still would want to make sure that whatever is output is a valid object. – Jeremy Cook Mar 31 '14 at 15:18
0

Thought to drop a line of code or two based on Torbjörn Hansson's golden answer:

public static class U
{
    private static readonly GeneralPurposeJsonJavaScriptEncodeConverter _generalEncoder = new GeneralPurposeJsonJavaScriptEncodeConverter();
    static public IHtmlString Js(this object obj) => new HtmlString(JsonConvert.SerializeObject(obj, _generalEncoder));

    private sealed class GeneralPurposeJsonJavaScriptEncodeConverter : JsonConverter //0
    {
        private static readonly Type TypeOfString = typeof(string);

        public override bool CanConvert(Type objectType) => objectType == TypeOfString;
        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) => reader.Value;
        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) => writer.WriteRawValue(Microsoft.Security.Application.Encoder.JavaScriptEncode((string) value, emitQuotes: true)); //1
    }
    //0 https://stackoverflow.com/a/28111588/863651   used when we need to burn raw json data directly inside a script element of our html like when we do when we use razor
    //1 note that the javascript encoder will leave nonenglish characters as they are and rightfully so   apparently the industry considers text in html attributes and inside
    //  html text blocks to be a battery for potential xss exploits and this is why the antixsslib applies html encoding on nonenglish characters there but not here   one
    //  could make the claim that using unicode escape sequences here for nonenglish characters could be potentionally useful if the clients receiving the server html response
    //  do not support utf8   however in our time and age clients that dont support utf8 are rarer than hens teeth so theres no point going this direction either
}

And here are some examples on how to use it (and when not to use it):

<span>
            @someStringWhichMightContainQuotes @* no need to use .Js() here *@
</span>

@* no need to use .Js() here *@
<input value="@someStringWhichMightContainQuotes" />

@* no need to use .Js() here either - this will work as intended automagically *@
@* notice however that we have to wrap the string in single-quotes *@
<button   onclick="Foobar( '@("abc  \"  '  ")'  )"> Text </button>

@* The resulting markup will be:
            <button onclick="Foobar(  'abc &quot; &#39; '  )"> Text </button>
Which will work as intended *@

And last but not least:

<script type="text/javascript">
    someJsController.Init({
        @* containerSelector: “#@(containerId.Js())”,  ← wrong  dont do this *@

        containerSelector: “#” + @(containerId.Js()),  @* ← correct  *@
        containerSelector2: @($"#{container2Id}".Js()),  @* ← even better do this for readability *@

        simpleString: @(Model.FilterCode.Js()), @* all these will serialize correctly *@
        someArray: @(Model.ColumnsNames.Js()), @* by simply calling the .js() method *@
        someNumeric: @(Model.SelectedId.Js()),
        complexCsharpObject: @(Model.complexCsharpObject.Js())
    });
</script>

Hope this helps.

XDS
  • 3,786
  • 2
  • 36
  • 56
0

If you're going to put the JSON inside <script> tags in an HTML file, you should escape the JSON for HTML:

<script type="text/javascript">
    var jsModel = @Html.Raw(JsonConvert.SerializeObject(Model), new JsonSerializerSettings { StringEscapeHandling = StringEscapeHandling.EscapeHtml })
</script>
ajarov
  • 149
  • 2
  • 5