Reflected XSS in Kendo DataSourceRequest object

[Sankar]

We have the below action in a controller for Kendo grid data population

1.public ActionResult GetCompanyDetails([DataSourceRequest] DataSourceRequest request)
2.{
3.    var companyDetails = BusinessLayer.GetCompaniesDetail();
4.    return Json(companyDetails.ToDataSourceResult(request), JsonRequestBehavior.AllowGet);
5.}

While running checkmarx scan the above method was identified for Reflected XSS vulnerability.

Method GetCompanyDetails at line 1 of wxy/xyz/Controllers/ABCController.cs gets user input for the request element. This element’s value then flows through the code without being properly sanitized or validated and is eventually displayed to the user in method GetCompanyDetails at line 4 of wxy/xyz/Controllers/ABCController.cs. This may enable a Cross-Site-Scripting attack.

How to sanitize the DataSourceRequest request object to fix this XSS issue?

[Aleksandar]

Hello Sankar,

The Grid component does its best to prevent XSS issues by encoding content during rendering. By default, the Grid encodes the content which is displayed within the cells, which means that if a user has submitted a script, the script will not execute but it will be displayed as a standard text. Have in mind that this encoding can be disabled by setting the column. Encoded configuration to false. Please make sure that the encoding in your grid is not disabled.

columns.Bound(x => x.Field).Encoded(false);

In addition to this, developers can also circumvent this via templates, in which case there should be taken extra care to encode any user data. Therefore, is that it is up to the developer to ensure that there are no XSS flaws.o You could use a library of your own choice to sanitize the request object. 

Having said all that, if you have experienced a case where XSS occurs via the DataSourceRequest could you please provide us with a sample solution of the scenario so we can investigate further?

Regards,
Aleksandar
Progress Telerik

[oscar]

Hi.

i read this post, but i would like to know if is there any way to make these errors that checkmarx are detecting to dessapear?.

How can i sanitize this kind of object "DataSourceRequest"? can you mention some AntiXss that allows me to sanitize this kind of object input? 

[Aleksandar]

Hello Oscar,

The DataSourceRequest object contains information on how the data should be paged, filtered, sorted, grouped. This information is further translated internally to System.Linq.Expressions.Expression class. In the end, the ToDataSourceResult executes a LINQ query based on the information contained DataSourceRequest object, passed to the action method. The DataSourceRequest object itself does not contain user-provided data and does not contain HTML.

Having said that you could sanitize the user input data using a library of your choice. I am afraid I cannot provide a suggestion on which library to use, though. I am attaching a sample project with Grid and DataSourceRequest. Could you please review it and let us know if you can replicate the security issue with it? This way we would be able to start inspecting further and be more helpful with a possible solution.

Regards,
Aleksandar
Progress Telerik

[Veerendra]

I am having the same Refelcted_XSS issue while returning the below json result:

 results.ToDataSourceResult(request)

Please suggest standard way to sanitize the json response that is being returned from the method.

 

Thanks,

Veerendra

[Veerendra]

Having same issue. The checkmarx complains about sanitizing the ToDatasourceResult(input Datarequest) coming in. Please suggest a way to do that.

Thanks,

Veerendra

[Aleksandar]

Hello Veerendra,

Thank you for reaching out. Can you please share the logs/results of the scan with us? We would need additional information in order to be able to investigate the case further. I would also ask you to review the sample project in my previous reply and let me know if you can replicate the security issue with it?

Regards,
Aleksandar
Progress Telerik

[Kevin]

Also having the same issue, The Checkmarx complains about sanitizing as well like the rest of these comments. Since no one had done it, I took your project you uploaded and ran it in our Chexkmarx system for you... Same Errors! We really need some assistance ASAP on this as it is delaying our production roll out. 

[Aleksandar]

Hello Kevin,

Thank you for providing the Checkmarx report. We are currently reviewing it and will get back to you once additional details are available.

Regards,
Aleksandar
Progress Telerik

[Aleksandar]

Hi Kevin,

The issue was discussed in the support thread posted on the same topic, however, as the topic is of interest to the community I am pasting the response provided here as well:

We have inspected the issue once again and couldn't find a thread in the scenarios. DataSourceRequest object is passed to the server to help sorting or to filter the data and is not responsible for returning any harmful data to the client. It returns the sorted or filtered data. In such a case the best approach that we recommend is sanitizing the data that is returned as JSON to the client - this is a general recommendation that is also applicable even when DataSourceRequest is not used.

Having this in mind we specify this warning observed in Checkmarx scan as false positive.

If, of course, you can recreate a case where XSS occurs via the DataSourceRequest could you please provide us with a sample solution of the scenario? We can then investigate further and take appropriate actions to resolve the issue.

Should you have any further questions or comments feel free to get back to us.

Regards,
Aleksandar
Progress Telerik

[Veerendra]

The Solution for Checkmarx to make the error go away is to sanitize the string paramters in the Datarequest. I Created a kendo Helper Function to do as follows. Hope this will be helpful to you all.

Kendo Helper:

----------------- Start -------------------

 public class KendoHelper
    {
        public static IList<AggregateDescriptor> SanitizeAggregates(IList<AggregateDescriptor> aggregateData)
        {
            if (aggregateData == null || !aggregateData.Any()) return aggregateData;
            foreach (var aggregate in aggregateData)
            {
                var descriptor = aggregate as AggregateDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    descriptor.Member = Sanitizer.GetSafeHtmlFragment(descriptor.Member);
                }
            }
            return aggregateData;
        }

        public static IList<SortDescriptor> SanitizeSorts(IList<SortDescriptor> sortData)
        {
            if (sortData == null || !sortData.Any()) return sortData;
            foreach (var sort in sortData)
            {
                var descriptor = sort as SortDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    descriptor.Member = Sanitizer.GetSafeHtmlFragment(descriptor.Member);
                }
            }
            return sortData;
        }

        public static IList<GroupDescriptor> SanitizeGroups(IList<GroupDescriptor> groupData)
        {
            if (groupData == null || !groupData.Any()) return groupData;
            foreach (var group in groupData)
            {
                var descriptor = group as GroupDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    descriptor.Member = Sanitizer.GetSafeHtmlFragment(descriptor.Member);
                }
            }
            return groupData;
        }


        public static IList<IFilterDescriptor> SanitizeFilters(IList<IFilterDescriptor> filters)
        {
            if (filters == null || !filters.Any()) return filters;
            foreach (var filter in filters)
            {
                var descriptor = filter as FilterDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    descriptor.Member = Sanitizer.GetSafeHtmlFragment(descriptor.Member);
                }
                else if (filter is CompositeFilterDescriptor)
                {
                    SanitizeFilters(((CompositeFilterDescriptor)filter).FilterDescriptors);
                }
            }
            return filters;
        }
    }

---------------- End -------------------------------------

 

Actual Kendo DataSourceRequest Sanitization using helper : 

 

 var sanitizedRequest = new DataSourceRequest();
            var filters = KendoHelper.SanitizeFilters(request.Filters);
            var sorts = KendoHelper.SanitizeSorts(request.Sorts);
            var groups = KendoHelper.SanitizeGroups(request.Groups);
            var aggregates = KendoHelper.SanitizeAggregates(request.Aggregates);
            sanitizedRequest.Aggregates = aggregates;
            sanitizedRequest.Filters = filters;
            sanitizedRequest.Groups = groups;
            sanitizedRequest.Page = request.Page;
            sanitizedRequest.PageSize = request.PageSize;
            sanitizedRequest.Sorts = sorts;
            var dataParameters = sanitizedRequest.ToDataParameters();

Thanks,

Veerendra

[Veerendra]

Check My Solution below. It worked on checkmarx after many do-overs.

 

Thanks,

Veerendra

 

[Kevin]

@Telerik, with the help of Veerendra and some digging on our side. I was able to get this working with ASP.NET Core, I would suggest that you guys add this workaround as an actual extension to your DatasourceRequest model. 

The solution Veerendra provided works great but does not work for Asp.Net Core because it uses AntiXSS and this is not supported by Core. This lead me down the road of finding a solution that would work for .NET Core and found you can use Ganss.XSS. So I updated Veerendra Code to look like this: 

public class KendoHelper
    {
        public static IList<AggregateDescriptor> SanitizeAggregates(IList<AggregateDescriptor> aggregateData)
        {
            if (aggregateData == null || !aggregateData.Any()) return aggregateData;
            foreach (var aggregate in aggregateData)
            {
                var descriptor = aggregate as AggregateDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    var sanitizer = new HtmlSanitizer();
                    descriptor.Member = sanitizer.Sanitize(descriptor.Member);
                }
            }
            return aggregateData;
        }

        public static IList<SortDescriptor> SanitizeSorts(IList<SortDescriptor> sortData)
        {
            if (sortData == null || !sortData.Any()) return sortData;
            foreach (var sort in sortData)
            {
                var descriptor = sort as SortDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    var sanitizer = new HtmlSanitizer();
                    descriptor.Member = sanitizer.Sanitize(descriptor.Member);
                }
            }
            return sortData;
        }

        public static IList<GroupDescriptor> SanitizeGroups(IList<GroupDescriptor> groupData)
        {
            if (groupData == null || !groupData.Any()) return groupData;
            foreach (var group in groupData)
            {
                var descriptor = group as GroupDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    var sanitizer = new HtmlSanitizer();
                    descriptor.Member = sanitizer.Sanitize(descriptor.Member);
                }
            }
            return groupData;
        }

        public static IList<IFilterDescriptor> SanitizeFilters(IList<IFilterDescriptor> filters)
        {
            if (filters == null || !filters.Any()) return filters;
            foreach (var filter in filters)
            {
                var descriptor = filter as FilterDescriptor;
                if (descriptor != null && descriptor.Member.GetType() == typeof(string))
                {
                    var sanitizer = new HtmlSanitizer();
                    descriptor.Member = sanitizer.Sanitize(descriptor.Member);
                }
                else if (filter is CompositeFilterDescriptor)
                {
                    SanitizeFilters(((CompositeFilterDescriptor)filter).FilterDescriptors);
                }
            }
            return filters;
        }


    }

 

I then created an Extension class for this call 

public static class ControllerExtensions
{
    public static DataSourceRequest SanitizeRequest(this DataSourceRequest request)
    {
        var sanitizedRequest = new DataSourceRequest();
        var filters = KendoHelper.SanitizeFilters(request.Filters);
        var sorts = KendoHelper.SanitizeSorts(request.Sorts);
        var groups = KendoHelper.SanitizeGroups(request.Groups);
        var aggregates = KendoHelper.SanitizeAggregates(request.Aggregates);
        sanitizedRequest.Aggregates = aggregates;
        sanitizedRequest.Filters = filters;
        sanitizedRequest.Groups = groups;
        sanitizedRequest.Page = request.Page;
        sanitizedRequest.PageSize = request.PageSize;
        sanitizedRequest.Sorts = sorts;
        return sanitizedRequest;
    }
}

 

Now this is really important because if you just do something like request.SanitizeRequest() this still fails the checkmarx security scanner... You can now alter each of the "Read" methods on the Grid to also send the AntiForgeryToken in the view like this:

<div class="row">
    <div class="col-12">
        @Html.AntiForgeryToken()

        @(
                    Html.Kendo().Grid<User>()
                    .Name("grid")
                    .Columns(columns =>
                    {
                        columns.Bound(c => c.EmailAddress);
                        columns.Bound(c => c.FirstName);
                        columns.Bound(c => c.LastName);
                        columns.Bound(c => c.Role);
                        columns.Command(c =>
                                {
                            c.Custom("Edit").Text("Edit").Click("edit").IconClass("k-icon k-i-edit");
                        });
                    })
                .Scrollable()
                .Groupable()
                .AutoBind(true)
                .Resizable(a => a.Columns(true))
                .Reorderable(a => a.Columns(true))
                .Selectable(s => s.Mode(GridSelectionMode.Single))
                .Filterable()
                .ColumnMenu()
                .Sortable()

                .Pageable(pageable => pageable
                        .Refresh(true)
                        .PageSizes(true)
                        .ButtonCount(5))
                .DataSource(dataSource => dataSource
                .Ajax()
                .PageSize(20)
                .Model(model => model.Id(a => a.Id))
                .Read(read => read.Action("GetUserList", "Users").Data("forgeryToken"))
                )
            )
    </div>
</div>

Now it is important to also add the following Javascript as well...

function forgeryToken() {
  return kendo.antiForgeryTokens();
}

 

Now let's look at what the controller looks like:

[HttpPost]
[ValidateAntiForgeryToken]
public IActionResult GetUserList([DataSourceRequest] DataSourceRequest request)
{
    return Json(_client.Users.GetUsers().ToDataSourceResult(request.SanitizeRequest()));
}

 

And this now will pass the Checkmarx security scan! Happy Coding!

[Plamen]

Hello,

Thank you for sharing your workaround - we will consider your recommendations.

Regards,
Plamen
Progress Telerik

[Vinoth]

Hi Kevin - 

 

I am still getting the checkmarx Reflected XSS issue, I have added AntiForgeryToken in Controller and View as well. any suggestions??

 

Regards,

Vinoth.T

[Plamen]

Hi,

We have discussed the issue internally and inspected it once again locally but we have not seen how it can actually be exploited. That is why at this moment we still consider it a false positive warning. If we are missing something please elaborate a possible vector or approach so we could reconsider the scenario once again.

Regards,
Plamen
Progress Telerik

[Jack]

In case anybody is looking for another way to get past the Checkmarx security flag, we found another method that works. This method will circumvent both the reflected XSS and stored XSS warnings from Checkmarx. (Both of which we believe are false positives.)

In the component, do something like:

        public async Task<IActionResult> OnPostAsync([DataSourceRequest] DataSourceRequest request)
        {
            // var someData = ...
            // 
            
            return new JsonCheckmarxResult(await someData.ToDataSourceResultAsync(request));
        }

 

Where our class JsonCheckmarxResult is defined as:

        /// <summary>
        /// Used to avoid reflected XSS and stored XSS security warnings from Checkmarx. There is no other reason why we are using this.
        /// </summary>
        public class JsonCheckmarxResult : ActionResult
        {
            private object _value;
            public JsonCheckmarxResult(object? value)
            {
                _value = value;
            }

            public override Task ExecuteResultAsync(ActionContext context)
            {
                if (context == null)
                {
                    throw new ArgumentNullException(nameof(context));
                }

                HttpResponse response = context.HttpContext.Response;
                response.ContentType = "application/json";
                return System.Text.Json.JsonSerializer.SerializeAsync(response.Body, _value);
            }
        }

 

The reason it works? I think it's because the Checkmarx scanner is looking for some magic string. Specifically, setting the ContentType explicitly seems to satisfy Checkmarx.

As far as we can tell, there is no real security issue here. This is just Checkmarx being Checkmarx :)