diff --git a/README.md b/README.md index 54a19f1a..fee30ea3 100644 --- a/README.md +++ b/README.md @@ -50,9 +50,15 @@ If it's not possible to add that attribute, you need to implement a custom [Cust Or provide a list of addtional types in the [DefaultDynamicLinqCustomTypeProvider.cs](https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/master/src/System.Linq.Dynamic.Core/CustomTypeProviders/DefaultDynamicLinqCustomTypeProvider.cs). ### v1.6.0-preview-01, 02, 03 -A breaking change is introduced in this version to solve CVE-2024-51417. +#### Change 1 It's not allowed anymore to call any methods on the `object` type. By default also the `ToString` and `Equals` methods are not allowed. To allow these methods set `AllowEqualsAndToStringMethodsOnObject` to `true` in the `ParsingConfig` and provide that config to all dynamic calls. +This is done to mitigate the risk of calling methods on the `object` type which could lead to security issues (CVE-2024-51417). + +#### Change 2 +By default the `RestrictOrderByToPropertyOrField` is now set to `true` in the `ParsingConfig`. +Which means that only properties and fields can be used in the `OrderBy` / `ThenBy`. +This is done to mitigate the risk of calling methods or other expressions in the `OrderBy` / `ThenBy` which could lead to security issues. --- diff --git a/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs b/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs index eb62a322..1ec0fea6 100644 --- a/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs +++ b/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs @@ -996,7 +996,9 @@ private Expression ParseIdentifier() if (isValid && shouldPrioritizeType) { - var keywordOrFunctionAllowed = !_usedForOrderBy || _usedForOrderBy && !_parsingConfig.RestrictOrderByToPropertyOrField; + var keywordOrFunctionAllowed = + !_usedForOrderBy || + (_usedForOrderBy && (_keywordsHelper.IsItOrRootOrParent(keywordOrType) || !_parsingConfig.RestrictOrderByToPropertyOrField)); if (!keywordOrFunctionAllowed) { throw ParseError(Res.UnknownPropertyOrField, _textParser.CurrentToken.Text, TypeHelper.GetTypeName(_it?.Type)); diff --git a/src/System.Linq.Dynamic.Core/Parser/IKeywordsHelper.cs b/src/System.Linq.Dynamic.Core/Parser/IKeywordsHelper.cs index c0d5b5ed..8ad51920 100644 --- a/src/System.Linq.Dynamic.Core/Parser/IKeywordsHelper.cs +++ b/src/System.Linq.Dynamic.Core/Parser/IKeywordsHelper.cs @@ -5,5 +5,7 @@ namespace System.Linq.Dynamic.Core.Parser; internal interface IKeywordsHelper { + bool IsItOrRootOrParent(AnyOf value); + bool TryGetValue(string text, out AnyOf value); } \ No newline at end of file diff --git a/src/System.Linq.Dynamic.Core/Parser/KeywordsHelper.cs b/src/System.Linq.Dynamic.Core/Parser/KeywordsHelper.cs index 380e3384..1e816384 100644 --- a/src/System.Linq.Dynamic.Core/Parser/KeywordsHelper.cs +++ b/src/System.Linq.Dynamic.Core/Parser/KeywordsHelper.cs @@ -84,6 +84,16 @@ public KeywordsHelper(ParsingConfig config) } } + public bool IsItOrRootOrParent(AnyOf value) + { + if (value.IsFirst) + { + return value.First is KEYWORD_IT or KEYWORD_ROOT or KEYWORD_PARENT or SYMBOL_IT or SYMBOL_PARENT or SYMBOL_ROOT; + } + + return false; + } + public bool TryGetValue(string text, out AnyOf value) { // 1. Try to get as constant-expression, keyword, symbol or functions diff --git a/src/System.Linq.Dynamic.Core/ParsingConfig.cs b/src/System.Linq.Dynamic.Core/ParsingConfig.cs index f13ee4ef..300ff594 100644 --- a/src/System.Linq.Dynamic.Core/ParsingConfig.cs +++ b/src/System.Linq.Dynamic.Core/ParsingConfig.cs @@ -307,11 +307,11 @@ public IQueryableAnalyzer QueryableAnalyzer public StringLiteralParsingType StringLiteralParsing { get; set; } = StringLiteralParsingType.Default; /// - /// When set to true, the parser will restrict the OrderBy method to only allow properties or fields. + /// When set to true, the parser will restrict the OrderBy and ThenBy methods to only allow properties or fields. If set to false, any expression is allowed. /// - /// Default value is false. + /// Default value is true. /// - public bool RestrictOrderByToPropertyOrField { get; set; } + public bool RestrictOrderByToPropertyOrField { get; set; } = true; /// /// When set to true, the parser will allow the use of the Equals(object obj), Equals(object objA, object objB), ReferenceEquals(object objA, object objB) and ToString() methods on the type. diff --git a/test/System.Linq.Dynamic.Core.Tests/EntitiesTests.OrderBy.cs b/test/System.Linq.Dynamic.Core.Tests/EntitiesTests.OrderBy.cs index 01324424..6c4d9373 100644 --- a/test/System.Linq.Dynamic.Core.Tests/EntitiesTests.OrderBy.cs +++ b/test/System.Linq.Dynamic.Core.Tests/EntitiesTests.OrderBy.cs @@ -1,4 +1,5 @@ using System.Linq.Dynamic.Core.Exceptions; +using System.Linq.Dynamic.Core.Parser; using System.Linq.Dynamic.Core.Tests.Helpers.Entities; using FluentAssertions; using Xunit; @@ -7,57 +8,58 @@ namespace System.Linq.Dynamic.Core.Tests; public partial class EntitiesTests { - [Fact] - public void Entities_OrderBy_RestrictOrderByIsFalse() + [Theory] + [InlineData("IIF(1 == 1, 1, 0)")] + [InlineData("np(Name, \"x\")")] + public void Entities_OrderBy_RestrictOrderByIsFalse_ShouldAllowAnyExpression(string expression) { + // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; + // Act - var resultBlogs = _context.Blogs.OrderBy(b => true).ToArray(); - var dynamicResultBlogs = _context.Blogs.OrderBy("IIF(1 == 1, 1, 0)").ToDynamicArray(); + Action action = () => _ = _context.Blogs.OrderBy(config, expression).ToDynamicArray(); // Assert - Assert.Equal(resultBlogs, dynamicResultBlogs); + action.Should().NotThrow(); } [Fact] - public void Entities_OrderBy_RestrictOrderByIsTrue_ValidExpressionShouldNotThrow() + public void Entities_OrderBy_RestrictOrderByIsTrue_NonRestrictedExpressionShouldNotThrow() { - // Arrange - var config = new ParsingConfig { RestrictOrderByToPropertyOrField = true }; - // Act 1 var resultBlogs = _context.Blogs.OrderBy(b => b.Name).ToArray(); - var dynamicResultBlogs = _context.Blogs.OrderBy(config, "Name").ToDynamicArray(); + var dynamicResultBlogs = _context.Blogs.OrderBy("Name").ToDynamicArray(); // Assert 1 Assert.Equal(resultBlogs, dynamicResultBlogs); // Act 2 var resultPosts = _context.Posts.OrderBy(p => p.Blog.Name).ToArray(); - var dynamicResultPosts = _context.Posts.OrderBy(config, "Blog.Name").ToDynamicArray(); + var dynamicResultPosts = _context.Posts.OrderBy("Blog.Name").ToDynamicArray(); // Assert 2 Assert.Equal(resultPosts, dynamicResultPosts); } - - [Fact] - public void Entities_OrderBy_RestrictOrderByIsTrue_InvalidExpressionShouldThrow() + + [Theory] + [InlineData("IIF(1 == 1, 1, 0)")] + [InlineData("np(Name, \"x\")")] + public void Entities_OrderBy_RestrictOrderByIsTrue_RestrictedExpressionShouldThrow(string expression) { - // Arrange - var config = new ParsingConfig { RestrictOrderByToPropertyOrField = true }; - // Act - Action action = () => _context.Blogs.OrderBy(config, "IIF(1 == 1, 1, 0)"); + Action action = () => _context.Blogs.OrderBy(expression); // Assert - action.Should().Throw().WithMessage("No property or field 'IIF' exists in type 'Blog'"); + action.Should().Throw().WithMessage("No property or field '*' exists in type 'Blog'"); } [Fact] public void Entities_OrderBy_NullPropagation_Int() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var resultBlogs = _context.Blogs.OrderBy(b => b.NullableInt ?? -1).ToArray(); - var dynamicResultBlogs = _context.Blogs.OrderBy("np(NullableInt, -1)").ToArray(); + var dynamicResultBlogs = _context.Blogs.OrderBy(config, "np(NullableInt, -1)").ToArray(); // Assert Assert.Equal(resultBlogs, dynamicResultBlogs); @@ -67,8 +69,9 @@ public void Entities_OrderBy_NullPropagation_Int() public void Entities_OrderBy_NullPropagation_String() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var resultBlogs = _context.Blogs.OrderBy(b => b.X ?? "_").ToArray(); - var dynamicResultBlogs = _context.Blogs.OrderBy("np(X, \"_\")").ToArray(); + var dynamicResultBlogs = _context.Blogs.OrderBy(config, "np(X, \"_\")").ToArray(); // Assert Assert.Equal(resultBlogs, dynamicResultBlogs); diff --git a/test/System.Linq.Dynamic.Core.Tests/ExpressionTests.cs b/test/System.Linq.Dynamic.Core.Tests/ExpressionTests.cs index 7190cbd1..1c00d1d6 100644 --- a/test/System.Linq.Dynamic.Core.Tests/ExpressionTests.cs +++ b/test/System.Linq.Dynamic.Core.Tests/ExpressionTests.cs @@ -1384,6 +1384,8 @@ public void ExpressionTests_IsNull_ThrowsException() [Fact] public void ExpressionTests_Indexer_Issue57() { + // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = true }; var rows = new List { new JObject {["Column1"] = "B", ["Column2"] = 1}, @@ -1392,9 +1394,11 @@ public void ExpressionTests_Indexer_Issue57() new JObject {["Column1"] = "A", ["Column2"] = 2} }; + // Act var expected = rows.OrderBy(x => x["Column1"]).ToList(); - var result = rows.AsQueryable().OrderBy(@"it[""Column1""]").ToList(); + var result = rows.AsQueryable().OrderBy(config, @"it[""Column1""]").ToList(); + // Assert Assert.Equal(expected, result); } @@ -1727,6 +1731,7 @@ public void ExpressionTests_NullPropagation_Method_WithDefaultValue() public void ExpressionTests_NullPropagating_DateTime() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var q = new[] { new { id = 1, date1 = (DateTime?) DateTime.Now, date2 = DateTime.Now.AddDays(-1) } @@ -1734,7 +1739,7 @@ public void ExpressionTests_NullPropagating_DateTime() // Act var result = q.OrderBy(x => x.date2).Select(x => x.id).ToArray(); - var resultDynamic = q.OrderBy("np(date1)").Select("id").ToDynamicArray(); + var resultDynamic = q.OrderBy(config, "np(date1)").Select("id").ToDynamicArray(); // Assert Check.That(resultDynamic).ContainsExactly(result); @@ -1744,6 +1749,7 @@ public void ExpressionTests_NullPropagating_DateTime() public void ExpressionTests_NullPropagation_NullableDateTime() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var q = new[] { new { id = 1, date1 = (DateTime?) DateTime.Now, date2 = DateTime.Now.AddDays(-1)} @@ -1751,7 +1757,7 @@ public void ExpressionTests_NullPropagation_NullableDateTime() // Act var result = q.OrderBy(x => x.date1.Value.Year).Select(x => x.id).ToArray(); - var resultDynamic = q.OrderBy("np(date1.Value.Year)").Select("id").ToDynamicArray(); + var resultDynamic = q.OrderBy(config, "np(date1.Value.Year)").Select("id").ToDynamicArray(); // Assert Check.That(resultDynamic).ContainsExactly(result); diff --git a/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderBy.cs b/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderBy.cs index a02661e6..f4dacb62 100644 --- a/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderBy.cs +++ b/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderBy.cs @@ -1,7 +1,9 @@ using System.Collections; using System.Collections.Generic; using System.Linq.Dynamic.Core.Exceptions; +using System.Linq.Dynamic.Core.Parser; using System.Linq.Dynamic.Core.Tests.Helpers.Models; +using FluentAssertions; using Xunit; namespace System.Linq.Dynamic.Core.Tests; @@ -36,12 +38,13 @@ public int Compare(object? x, object? y) public void OrderBy_Dynamic_NullPropagation_Int() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var testList = User.GenerateSampleModels(2); var qry = testList.AsQueryable(); // Act var orderBy = testList.OrderBy(x => x.NullableInt ?? -1).ToArray(); - var orderByDynamic = qry.OrderBy("np(NullableInt, -1)").ToArray(); + var orderByDynamic = qry.OrderBy(config, "np(NullableInt, -1)").ToArray(); // Assert Assert.Equal(orderBy, orderByDynamic); @@ -51,12 +54,13 @@ public void OrderBy_Dynamic_NullPropagation_Int() public void OrderBy_Dynamic_NullPropagation_String() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var testList = User.GenerateSampleModels(2); var qry = testList.AsQueryable(); // Act var orderBy = testList.OrderBy(x => x.NullableString ?? "_").ToArray(); - var orderByDynamic = qry.OrderBy("np(NullableString, \"_\")").ToArray(); + var orderByDynamic = qry.OrderBy(config, "np(NullableString, \"_\")").ToArray(); // Assert Assert.Equal(orderBy, orderByDynamic); @@ -66,12 +70,13 @@ public void OrderBy_Dynamic_NullPropagation_String() public void OrderBy_Dynamic_NullPropagation_NestedObject() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var testList = User.GenerateSampleModels(2); var qry = testList.AsQueryable(); // Act var orderBy = testList.OrderBy(x => x.Profile?.Age ?? -1).ToArray(); - var orderByDynamic = qry.OrderBy("np(Profile.Age, -1)").ToArray(); + var orderByDynamic = qry.OrderBy(config, "np(Profile.Age, -1)").ToArray(); // Assert Assert.Equal(orderBy, orderByDynamic); @@ -81,6 +86,7 @@ public void OrderBy_Dynamic_NullPropagation_NestedObject() public void OrderBy_Dynamic_NullPropagation_NestedObject_Query() { // Arrange + var config = new ParsingConfig { RestrictOrderByToPropertyOrField = false }; var qry = User.GenerateSampleModels(2) .Select(u => new { @@ -93,7 +99,7 @@ public void OrderBy_Dynamic_NullPropagation_NestedObject_Query() .AsQueryable(); // Act - var orderByDynamic = qry.OrderBy("np(X.Age, -1)").ToArray(); + var orderByDynamic = qry.OrderBy(config, "np(X.Age, -1)").ToArray(); // Assert Assert.NotNull(orderByDynamic); @@ -238,4 +244,24 @@ public void OrderBy_Dynamic_Exceptions() Assert.Throws(() => qry.OrderBy("")); Assert.Throws(() => qry.OrderBy(" ")); } + + [Theory] + [InlineData(KeywordsHelper.KEYWORD_IT)] + [InlineData(KeywordsHelper.SYMBOL_IT)] + [InlineData(KeywordsHelper.KEYWORD_ROOT)] + [InlineData(KeywordsHelper.SYMBOL_ROOT)] + [InlineData("\"User\" + \"Name\"")] + [InlineData("\"User\" + \"Name\" asc")] + [InlineData("\"User\" + \"Name\" desc")] + public void OrderBy_RestrictOrderByIsTrue_NonRestrictedExpressionShouldNotThrow(string expression) + { + // Arrange + var queryable = User.GenerateSampleModels(3).AsQueryable(); + + // Act + Action action = () => _ = queryable.OrderBy(expression); + + // Assert 2 + action.Should().NotThrow(); + } } \ No newline at end of file diff --git a/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderByDescending.cs b/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderByDescending.cs index e4e6e074..d7712a2f 100644 --- a/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderByDescending.cs +++ b/test/System.Linq.Dynamic.Core.Tests/QueryableTests.OrderByDescending.cs @@ -1,5 +1,4 @@ -using System.Linq.Dynamic.Core.Exceptions; -using System.Linq.Dynamic.Core.Tests.Helpers.Models; +using System.Linq.Dynamic.Core.Tests.Helpers.Models; using Xunit; namespace System.Linq.Dynamic.Core.Tests