diff --git a/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs b/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs index f2d108cc5..c2132a961 100644 --- a/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs +++ b/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs @@ -1,8 +1,10 @@ -using Microsoft.AspNetCore.Mvc.Filters; +using Grand.Module.Api.Constants; +using Grand.Module.Api.Queries; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; using System.Linq.Dynamic.Core; -using Microsoft.AspNetCore.Http; -using Grand.Module.Api.Constants; +using System.Linq.Dynamic.Core.Exceptions; namespace Grand.Module.Api.Attributes; @@ -14,34 +16,62 @@ public override void OnActionExecuted(ActionExecutedContext context) if (context.Result is not ObjectResult result || result.Value == null) return; - if (result.Value is IQueryable queryable) + if (result.Value is not IQueryable queryable) + return; + + try + { + result.Value = ApplyQueryOptions(queryable, context.HttpContext.Request.Query); + } + catch (ApiQueryOptionException ex) + { + //a rejected query option is the client's mistake, not ours - this filter runs after the + //action, so without this the exception would leave the pipeline as a 500 + context.Result = new BadRequestObjectResult(new { error = ex.Message }); + } + catch (ParseException ex) { - queryable = ApplyQueryOptions(queryable, context.HttpContext.Request.Query, context.HttpContext.Response); - result.Value = queryable; + context.Result = new BadRequestObjectResult(new { error = $"$filter could not be parsed: {ex.Message}" }); } } - private static IQueryable ApplyQueryOptions(IQueryable queryable, IQueryCollection query, HttpResponse response) + private static IQueryable ApplyQueryOptions(IQueryable queryable, IQueryCollection query) { + var elementType = queryable.ElementType; + if (query.TryGetValue("$filter", out var filter)) - queryable = queryable.Where(filter.ToString()); + { + ApiQueryOptions.ValidateFilter(filter.ToString(), elementType); + queryable = queryable.Where(ApiQueryOptions.FilterConfig, filter.ToString()); + } if (query.TryGetValue("$orderby", out var orderBy)) - queryable = queryable.OrderBy(orderBy.ToString()); + queryable = queryable.OrderBy(ApiQueryOptions.FilterConfig, + ApiQueryOptions.ParseOrderBy(orderBy.ToString(), elementType)); if (query.TryGetValue("$select", out var select)) - queryable = queryable.Select($"new({select})"); + queryable = queryable.Select(ApiQueryOptions.SelectConfig, + ApiQueryOptions.ParseSelect(select.ToString(), elementType)); + + if (query.TryGetValue("$skip", out var skipValue)) + { + if (!int.TryParse(skipValue, out var skip) || skip < 0) + throw new ApiQueryOptionException("$skip must be a non-negative integer"); - if (query.TryGetValue("$skip", out var skipValue) && int.TryParse(skipValue, out var skip)) queryable = queryable.Skip(skip); + } - if (query.TryGetValue("$top", out var topValue) && int.TryParse(topValue, out var top)) + if (query.TryGetValue("$top", out var topValue)) { - top = Math.Min(top, Configurations.MaxLimit); - queryable = queryable.Take(top); + if (!int.TryParse(topValue, out var top) || top < 0) + throw new ApiQueryOptionException("$top must be a non-negative integer"); + + queryable = queryable.Take(Math.Min(top, Configurations.MaxLimit)); } else + { queryable = queryable.Take(Configurations.MaxLimit); + } return queryable; } diff --git a/src/Modules/Grand.Module.Api/Queries/ApiQueryOptions.cs b/src/Modules/Grand.Module.Api/Queries/ApiQueryOptions.cs new file mode 100644 index 000000000..c06dcdc76 --- /dev/null +++ b/src/Modules/Grand.Module.Api/Queries/ApiQueryOptions.cs @@ -0,0 +1,203 @@ +using System.Linq.Dynamic.Core; +using System.Linq.Dynamic.Core.CustomTypeProviders; +using System.Reflection; +using System.Text.RegularExpressions; + +namespace Grand.Module.Api.Queries; + +/// +/// Raised when a client sends a query option the API refuses to run. Mapped to 400 by the caller; +/// it is never an internal error, so it must not surface as 500. +/// +public class ApiQueryOptionException(string message) : Exception(message); + +/// +/// Parses and restricts the OData-like query options. +/// $filter reaches a real expression parser, so it is fenced on three sides: a parsing config that +/// denies types, context keywords and object construction; a length limit; and a whitelist that +/// accepts only members of the projected model. $orderby and $select never reach the parser as raw +/// text - they are read as field lists and rebuilt here. +/// +public static class ApiQueryOptions +{ + /// + /// Long enough for a realistic filter, short enough that a pathological expression cannot make + /// the parser the slow part of the request. + /// + public const int MaxFilterLength = 512; + + public const int MaxFields = 50; + + /// + /// Methods a filter may name. Everything here is a cheap, side-effect free string or text + /// operation; nothing that reflects, constructs, or walks a collection. + /// + private static readonly HashSet AllowedMethods = new(StringComparer.OrdinalIgnoreCase) { + "Contains", "StartsWith", "EndsWith", "ToLower", "ToUpper", "Trim", "Length", "Equals" + }; + + /// + /// Operators and literals the parser understands that are not members of the model. + /// + private static readonly HashSet Keywords = new(StringComparer.OrdinalIgnoreCase) { + "and", "or", "not", "true", "false", "null", "iif" + }; + + private static readonly Regex Identifier = new(@"[A-Za-z_][A-Za-z0-9_]*", RegexOptions.Compiled); + + /// + /// String literals hold user text, not member names, so they are removed before the whitelist + /// runs - otherwise a product named "Password" would fail its own search. + /// + private static readonly Regex StringLiteral = new(@"""(?:[^""\\]|\\.)*""|'(?:[^'\\]|\\.)*'", RegexOptions.Compiled); + + /// + /// Denies everything the parser can reach outside the model: no type resolution, no `it`/`root` + /// context keywords, no `new`, no assembly probing, no Equals/ToString on object. + /// + public static ParsingConfig FilterConfig { get; } = new() { + AreContextKeywordsEnabled = false, + AllowNewToEvaluateAnyType = false, + DisallowNewKeyword = true, + ResolveTypesBySimpleName = false, + SupportCastingToFullyQualifiedTypeAsString = false, + LoadAdditionalAssembliesFromCurrentDomainBaseDirectory = false, + AllowEqualsAndToStringMethodsOnObject = false, + RestrictOrderByToPropertyOrField = true, + CustomTypeProvider = new NoCustomTypesProvider() + }; + + /// + /// $select is rebuilt from validated field names, so `new` has to be available for that one + /// projection - and for nothing else. + /// + public static ParsingConfig SelectConfig { get; } = new() { + AreContextKeywordsEnabled = false, + AllowNewToEvaluateAnyType = false, + ResolveTypesBySimpleName = false, + SupportCastingToFullyQualifiedTypeAsString = false, + LoadAdditionalAssembliesFromCurrentDomainBaseDirectory = false, + AllowEqualsAndToStringMethodsOnObject = false, + RestrictOrderByToPropertyOrField = true, + CustomTypeProvider = new NoCustomTypesProvider() + }; + + /// + /// Checks that a filter names nothing outside the model it runs against. + /// + public static void ValidateFilter(string filter, Type elementType) + { + if (string.IsNullOrWhiteSpace(filter)) + throw new ApiQueryOptionException("$filter is empty"); + + if (filter.Length > MaxFilterLength) + throw new ApiQueryOptionException($"$filter exceeds {MaxFilterLength} characters"); + + var members = MemberNames(elementType); + var expression = StringLiteral.Replace(filter, " "); + + foreach (Match match in Identifier.Matches(expression)) + { + var name = match.Value; + if (Keywords.Contains(name) || AllowedMethods.Contains(name) || members.Contains(name)) + continue; + + throw new ApiQueryOptionException($"'{name}' is not a queryable field of {elementType.Name}"); + } + } + + /// + /// Reads "Field asc, Other desc" and gives it back with every field checked against the model. + /// + public static string ParseOrderBy(string orderBy, Type elementType) + { + var members = MemberNames(elementType); + var parts = SplitFields(orderBy, "$orderby"); + var ordering = new List(parts.Count); + + foreach (var part in parts) + { + var tokens = part.Split(' ', StringSplitOptions.RemoveEmptyEntries); + if (tokens.Length > 2) + throw new ApiQueryOptionException($"'{part}' is not a valid $orderby entry"); + + if (!members.Contains(tokens[0])) + throw new ApiQueryOptionException($"'{tokens[0]}' is not a queryable field of {elementType.Name}"); + + var direction = tokens.Length == 2 ? tokens[1].ToLowerInvariant() : "asc"; + if (direction != "asc" && direction != "desc") + throw new ApiQueryOptionException($"'{tokens[1]}' is not a sort direction"); + + ordering.Add($"{tokens[0]} {direction}"); + } + + return string.Join(", ", ordering); + } + + /// + /// Reads a field list and builds the projection itself, so no client text reaches the parser. + /// + public static string ParseSelect(string select, Type elementType) + { + var members = MemberNames(elementType); + var fields = SplitFields(select, "$select"); + + foreach (var field in fields) + if (!members.Contains(field)) + throw new ApiQueryOptionException($"'{field}' is not a queryable field of {elementType.Name}"); + + return $"new({string.Join(", ", fields)})"; + } + + private static List SplitFields(string value, string option) + { + if (string.IsNullOrWhiteSpace(value)) + throw new ApiQueryOptionException($"{option} is empty"); + + var fields = value.Split(',', StringSplitOptions.RemoveEmptyEntries) + .Select(x => x.Trim()) + .Where(x => x.Length > 0) + .ToList(); + + if (fields.Count == 0) + throw new ApiQueryOptionException($"{option} is empty"); + + if (fields.Count > MaxFields) + throw new ApiQueryOptionException($"{option} lists more than {MaxFields} fields"); + + return fields; + } + + private static HashSet MemberNames(Type elementType) + { + return new HashSet( + elementType.GetProperties(BindingFlags.Public | BindingFlags.Instance).Select(x => x.Name), + StringComparer.OrdinalIgnoreCase); + } + + /// + /// Leaves the parser with no types to resolve at all. + /// + private class NoCustomTypesProvider : IDynamicLinqCustomTypeProvider + { + public HashSet GetCustomTypes() + { + return []; + } + + public Dictionary> GetExtensionMethods() + { + return []; + } + + public Type ResolveType(string typeName) + { + return null; + } + + public Type ResolveTypeBySimpleName(string simpleTypeName) + { + return null; + } + } +} diff --git a/src/Tests/Grand.Module.Api.Tests/Queries/ApiQueryOptionsTests.cs b/src/Tests/Grand.Module.Api.Tests/Queries/ApiQueryOptionsTests.cs new file mode 100644 index 000000000..2ac02abb4 --- /dev/null +++ b/src/Tests/Grand.Module.Api.Tests/Queries/ApiQueryOptionsTests.cs @@ -0,0 +1,101 @@ +using Grand.Module.Api.DTOs.Catalog; +using Grand.Module.Api.Queries; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Linq.Dynamic.Core; +using Assert = Microsoft.VisualStudio.TestTools.UnitTesting.Assert; + +namespace Grand.Module.Api.Tests.Queries; + +[TestClass] +public class ApiQueryOptionsTests +{ + private static readonly Type ElementType = typeof(ProductDto); + + [TestMethod] + public void Filter_AcceptsAFieldOfTheModel() + { + ApiQueryOptions.ValidateFilter("Name.Contains(\"shirt\") and Published == true", ElementType); + } + + [TestMethod] + public void Filter_KeepsALiteralOutOfTheFieldCheck() + { + //the text is data, not a member name - a product called "Password" must stay searchable + ApiQueryOptions.ValidateFilter("Name == \"Password\"", ElementType); + } + + [TestMethod] + [DataRow("PasswordHash != null", DisplayName = "field the model does not expose")] + [DataRow("it.GetType().Assembly != null", DisplayName = "reflection through the context keyword")] + [DataRow("\"\".GetType().Assembly.GetTypes().Length > 0", DisplayName = "type walk from a literal")] + [DataRow("System.IO.File.ReadAllText(\"appsettings.json\") != null", DisplayName = "fully qualified type")] + [DataRow("new(Name as X).X != null", DisplayName = "object construction")] + [DataRow("Name.Equals(Name, StringComparison.Ordinal)", DisplayName = "type reference in an argument")] + public void Filter_RejectsAnythingOutsideTheModel(string filter) + { + Assert.ThrowsExactly(() => ApiQueryOptions.ValidateFilter(filter, ElementType)); + } + + [TestMethod] + public void Filter_RejectsAnOverlongExpression() + { + var filter = string.Join(" or ", Enumerable.Repeat("Name == \"x\"", 100)); + + var ex = Assert.ThrowsExactly( + () => ApiQueryOptions.ValidateFilter(filter, ElementType)); + + StringAssert.Contains(ex.Message, "characters"); + } + + /// + /// The whitelist decides what runs; this checks the parser it runs on is locked down too, so a + /// future change to the whitelist cannot quietly re-open type access. + /// + [TestMethod] + public void FilterConfig_RefusesToResolveTypes() + { + var source = new[] { new ProductDto { Name = "shirt" } }.AsQueryable(); + + Assert.ThrowsExactly( + () => source.Where(ApiQueryOptions.FilterConfig, "it.GetType().Name == \"ProductDto\"").ToList()); + } + + [TestMethod] + public void OrderBy_NormalizesDirectionAndDefaultsToAscending() + { + Assert.AreEqual("Name asc, Sku desc", ApiQueryOptions.ParseOrderBy("Name, Sku DESC", ElementType)); + } + + [TestMethod] + [DataRow("PasswordHash")] + [DataRow("Name sideways")] + [DataRow("Name asc extra")] + [DataRow("")] + public void OrderBy_RejectsWhatItCannotVerify(string orderBy) + { + Assert.ThrowsExactly(() => ApiQueryOptions.ParseOrderBy(orderBy, ElementType)); + } + + [TestMethod] + public void Select_BuildsTheProjectionFromCheckedFields() + { + Assert.AreEqual("new(Id, Name)", ApiQueryOptions.ParseSelect("Id, Name", ElementType)); + } + + [TestMethod] + [DataRow("PasswordHash", DisplayName = "field the model does not expose")] + [DataRow("Name as Alias", DisplayName = "expression rather than a field")] + [DataRow("it.GetType()", DisplayName = "reflection")] + public void Select_RejectsAnythingThatIsNotAPlainField(string select) + { + Assert.ThrowsExactly(() => ApiQueryOptions.ParseSelect(select, ElementType)); + } + + [TestMethod] + public void Select_RejectsMoreFieldsThanTheLimit() + { + var select = string.Join(",", Enumerable.Range(0, ApiQueryOptions.MaxFields + 1).Select(_ => "Name")); + + Assert.ThrowsExactly(() => ApiQueryOptions.ParseSelect(select, ElementType)); + } +}