CODEWITHSUNDEEP
c#asp.net-mvcsecurityxsscheckmarx
decoder
decoder

Reputation: 926

ASP.Net MVC App Stored XSS vulnerability reported by Checkmarx

Code has been analyzed by Checkmarx and reported the following issue:

Method Load_Bank at line ** gets data from the database, for the Where element. This element’s value then flows through the code without being properly filtered or encoded and is eventually displayed to the user in method Bank_Read at line * of SomeController.cs. This may enable a Stored Cross-SiteScripting attack. 

internal IEnumerable<BankDTO> Load_Bank()
{
    using (var Container = new EBookletEntities())
    {
        var query = from r in Container.Gen_Bank.AsNoTracking()
                    where r.IsDeleted != true
                    select new Gen_BankDTO
                    {
                        Id = r.Id,
                        Name = r.Name
                    };
        return query.ToList<BankDTO>();
    }
}

Below is the controller code

using (var bll = new BankBLL())
{
    var item = bll.Load_Bank();
    var model = item.Select(r => new BVM()
        {
            Id = r.Id,
            Name = HttpUtility.HtmlEncode(r.Name)
        }).ToList();

    return Json(model.ToDataSourceResult(request), "application/json", System.Text.Encoding.UTF8, JsonRequestBehavior.AllowGet);

}

Checkmarx source:

where r.IsDeleted != true

Destination:

return Json(model.ToDataSourceResult(request), "application/json", System.Text.Encoding.UTF8, JsonRequestBehavior.AllowGet);

I wonder if there is really a Stored XSS issue or Checkmarx reported it false?

How to resolve the Checkmarx issue?

Upvotes: 1

Views: 2340

Answers (1)

Gabor Lengyel
Gabor Lengyel

Reputation: 15570

This is not exploitable, because the response type is application/json. Even if there was a valid xss attack with a script tag, no modern browser will execute that in a response with an application/json content type.

Also Id I guess is a number or uuid and Name is html encoded, which you could argue is for defense in depth, but it only actually needs to be encoded for json, which it is inherently.

You can mark this as not exploitable in Checkmarx.

Also note that returning a json array in a GET request is still not considered good practice because of an old attack called json hijacking. However, this is not exploitable anymore in modern browsers, so I would not say it's vulnerable anymore, except in IE9, which might still be in use unfortunately.

Upvotes: 3

Related Questions