Browse Source

fix(ddd): restrict dynamic sorting selectors to property/field access

hooks System.Linq.Dynamic.Core's QueryOptimizer so OrderBy / ThenBy
selectors derived from ISortedResultRequest.Sorting are constrained to
plain property or field access; anything else throws AbpValidationException
pull/25617/head
maliming 4 months ago
parent
commit
4933a23f6f
No known key found for this signature in database GPG Key ID: A646B9CB645ECEA4
  1. 5
      framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/AbpDddApplicationModule.cs
  2. 95
      framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AbpDynamicSortingGuard.cs
  3. 3
      framework/test/Volo.Abp.Ddd.Application.Tests/Volo.Abp.Ddd.Application.Tests.abppkg
  4. 18
      framework/test/Volo.Abp.Ddd.Application.Tests/Volo.Abp.Ddd.Application.Tests.csproj
  5. 12
      framework/test/Volo.Abp.Ddd.Application.Tests/Volo/Abp/Application/AbpDddApplicationTestBase.cs
  6. 12
      framework/test/Volo.Abp.Ddd.Application.Tests/Volo/Abp/Application/AbpDddApplicationTestModule.cs
  7. 131
      framework/test/Volo.Abp.Ddd.Application.Tests/Volo/Abp/Application/Services/AbpDynamicSortingGuard_Tests.cs

5
framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/AbpDddApplicationModule.cs

@ -30,6 +30,11 @@ namespace Volo.Abp.Application;
)]
public class AbpDddApplicationModule : AbpModule
{
public override void PreConfigureServices(ServiceConfigurationContext context)
{
AbpDynamicSortingGuard.Install();
}
public override void ConfigureServices(ServiceConfigurationContext context)
{
Configure<AbpApiDescriptionModelOptions>(options =>

95
framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AbpDynamicSortingGuard.cs

@ -0,0 +1,95 @@
using System;
using System.Linq;
using System.Linq.Dynamic.Core;
using System.Linq.Expressions;
using System.Runtime.CompilerServices;
using Volo.Abp.Validation;
[assembly: InternalsVisibleTo("Volo.Abp.Ddd.Application.Tests")]
namespace Volo.Abp.Application.Services;
/// <summary>
/// Framework infrastructure. Hooks <see cref="ExtensibilityPoint.QueryOptimizer"/> so
/// every OrderBy / ThenBy expression built from a user-supplied sorting string is
/// constrained to plain property or field access. Methods, comparisons, ternaries
/// and constants in the sort key are rejected with <see cref="AbpValidationException"/>.
/// </summary>
internal static class AbpDynamicSortingGuard
{
private static readonly object InstallLock = new();
private static Func<Expression, Expression>? _activeOptimizer;
public static void Install()
{
lock (InstallLock)
{
var current = ExtensibilityPoint.QueryOptimizer;
if (_activeOptimizer != null && ReferenceEquals(current, _activeOptimizer))
{
return;
}
var previous = current;
_activeOptimizer = expression =>
{
new OrderByMethodVisitor().Visit(expression);
return previous != null ? previous(expression) : expression;
};
ExtensibilityPoint.QueryOptimizer = _activeOptimizer;
}
}
internal static void Reset()
{
lock (InstallLock)
{
if (ReferenceEquals(ExtensibilityPoint.QueryOptimizer, _activeOptimizer))
{
ExtensibilityPoint.QueryOptimizer = null;
}
_activeOptimizer = null;
}
}
private sealed class OrderByMethodVisitor : ExpressionVisitor
{
protected override Expression VisitMethodCall(MethodCallExpression node)
{
if (node.Method.DeclaringType == typeof(Queryable) &&
IsOrderByMethod(node.Method.Name) &&
node.Arguments.Count >= 2 &&
node.Arguments[1] is UnaryExpression { Operand: LambdaExpression lambda })
{
new PropertyOnlySelectorVisitor().Visit(lambda.Body);
}
return base.VisitMethodCall(node);
}
private static bool IsOrderByMethod(string name)
{
return name == nameof(Queryable.OrderBy)
|| name == nameof(Queryable.OrderByDescending)
|| name == nameof(Queryable.ThenBy)
|| name == nameof(Queryable.ThenByDescending);
}
}
private sealed class PropertyOnlySelectorVisitor : ExpressionVisitor
{
private const string Message = "Sorting expression is not supported.";
protected override Expression VisitMethodCall(MethodCallExpression node)
=> throw new AbpValidationException(Message);
protected override Expression VisitBinary(BinaryExpression node)
=> throw new AbpValidationException(Message);
protected override Expression VisitConditional(ConditionalExpression node)
=> throw new AbpValidationException(Message);
protected override Expression VisitConstant(ConstantExpression node)
=> throw new AbpValidationException(Message);
}
}

3
framework/test/Volo.Abp.Ddd.Application.Tests/Volo.Abp.Ddd.Application.Tests.abppkg

@ -0,0 +1,3 @@
{
"role": "lib.test"
}

18
framework/test/Volo.Abp.Ddd.Application.Tests/Volo.Abp.Ddd.Application.Tests.csproj

@ -0,0 +1,18 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\..\..\common.test.props" />
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
<RootNamespace />
</PropertyGroup>
<ItemGroup>
<ProjectReference Include="..\..\src\Volo.Abp.Autofac\Volo.Abp.Autofac.csproj" />
<ProjectReference Include="..\..\src\Volo.Abp.Ddd.Application\Volo.Abp.Ddd.Application.csproj" />
<ProjectReference Include="..\..\src\Volo.Abp.ExceptionHandling\Volo.Abp.ExceptionHandling.csproj" />
<ProjectReference Include="..\AbpTestBase\AbpTestBase.csproj" />
<PackageReference Include="Microsoft.NET.Test.Sdk" />
</ItemGroup>
</Project>

12
framework/test/Volo.Abp.Ddd.Application.Tests/Volo/Abp/Application/AbpDddApplicationTestBase.cs

@ -0,0 +1,12 @@
using Volo.Abp.Modularity;
using Volo.Abp.Testing;
namespace Volo.Abp.Application;
public abstract class AbpDddApplicationTestBase : AbpIntegratedTest<AbpDddApplicationTestModule>
{
protected override void SetAbpApplicationCreationOptions(AbpApplicationCreationOptions options)
{
options.UseAutofac();
}
}

12
framework/test/Volo.Abp.Ddd.Application.Tests/Volo/Abp/Application/AbpDddApplicationTestModule.cs

@ -0,0 +1,12 @@
using Volo.Abp.Autofac;
using Volo.Abp.ExceptionHandling;
using Volo.Abp.Modularity;
namespace Volo.Abp.Application;
[DependsOn(typeof(AbpAutofacModule))]
[DependsOn(typeof(AbpDddApplicationModule))]
[DependsOn(typeof(AbpExceptionHandlingModule))]
public class AbpDddApplicationTestModule : AbpModule
{
}

131
framework/test/Volo.Abp.Ddd.Application.Tests/Volo/Abp/Application/Services/AbpDynamicSortingGuard_Tests.cs

@ -0,0 +1,131 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Dynamic.Core;
using Shouldly;
using Volo.Abp.Validation;
using Xunit;
namespace Volo.Abp.Application.Services;
public class AbpDynamicSortingGuard_Tests : AbpDddApplicationTestBase
{
private readonly IQueryable<FakeUser> _users;
public AbpDynamicSortingGuard_Tests()
{
_users = new List<FakeUser>
{
new() { Name = "alice", Age = 30, PasswordHash = "AQAAhash_alice", Tenant = new FakeTenant { Name = "acme" } },
new() { Name = "bob", Age = 25, PasswordHash = "BQAAhash_bob", Tenant = new FakeTenant { Name = "beta" } },
new() { Name = "carl", Age = 40, PasswordHash = "CQAAhash_carl", Tenant = new FakeTenant { Name = "corp" } },
}.AsQueryable();
}
[Theory]
[InlineData("Name")]
[InlineData("Name desc")]
[InlineData("Name asc, PasswordHash desc")]
[InlineData("Age desc")] // value type → EF/Dynamic.Core wraps selector in Convert(MemberAccess, object)
[InlineData("Tenant.Name")] // chained MemberAccess
[InlineData("Tenant.Name desc, Age asc")] // mixed chained + value-type, multi-column
[InlineData("Name.Length desc")] // Length is a property getter, not a method call
public void Should_Accept_Plain_Property_Sorting(string sorting)
{
Should.NotThrow(() => _users.OrderBy(sorting).ToList());
}
[Theory]
[InlineData("PasswordHash.Substring(0,1) desc")]
[InlineData("PasswordHash.StartsWith(\"A\") desc")]
[InlineData("PasswordHash.Contains(\"hash\") desc")]
[InlineData("Name asc, PasswordHash.Substring(0,1) desc")] // multi-column with attack in 2nd
public void Should_Reject_Method_Call_On_Property(string sorting)
{
Should.Throw<AbpValidationException>(() => _users.OrderBy(sorting).ToList())
.Message.ShouldBe("Sorting expression is not supported.");
}
[Theory]
[InlineData("(PasswordHash == \"AQAA\") desc")]
[InlineData("(PasswordHash > \"M\") desc")]
[InlineData("(PasswordHash != \"AQAA\") asc")]
public void Should_Reject_Binary_Expressions(string sorting)
{
Should.Throw<AbpValidationException>(() => _users.OrderBy(sorting).ToList())
.Message.ShouldBe("Sorting expression is not supported.");
}
[Fact]
public void Should_Not_Affect_Where_Expressions()
{
// The guard only inspects Queryable.OrderBy/ThenBy nodes. Where with the same
// sub-expression is left alone (Where is a separate vulnerability class).
Should.NotThrow(() => _users.Where("PasswordHash.StartsWith(\"A\")").ToList());
}
[Fact]
public void Install_Chains_Existing_QueryOptimizer()
{
// Reset guard state so Install() actually re-installs and exercises the
// `previous != null ? previous(expression) : expression` branch.
AbpDynamicSortingGuard.Reset();
try
{
var preExistingFired = false;
ExtensibilityPoint.QueryOptimizer = e =>
{
preExistingFired = true;
return e;
};
AbpDynamicSortingGuard.Install();
_users.OrderBy("Name").ToList();
preExistingFired.ShouldBeTrue();
}
finally
{
// Leave the AppDomain with a single-layer guard. Reset() clears whatever
// we wrapped in this test; Install() then puts a fresh guard on top of
// an empty QueryOptimizer — never double-wraps an existing guard.
AbpDynamicSortingGuard.Reset();
AbpDynamicSortingGuard.Install();
}
}
[Fact]
public void Install_Reinstalls_When_QueryOptimizer_Was_Replaced()
{
// Simulate someone (e.g. a test teardown, another module) overwriting our
// optimizer. The next Install() must detect the mismatch and wrap again.
try
{
ExtensibilityPoint.QueryOptimizer = e => e; // not our wrapper
AbpDynamicSortingGuard.Install();
// Guard must be active again — attack payload still gets rejected.
Should.Throw<AbpValidationException>(() =>
_users.OrderBy("PasswordHash.Substring(0,1) desc").ToList());
}
finally
{
AbpDynamicSortingGuard.Reset();
AbpDynamicSortingGuard.Install();
}
}
private class FakeUser
{
public string Name { get; set; } = "";
public int Age { get; set; }
public string PasswordHash { get; set; } = "";
public FakeTenant Tenant { get; set; } = new();
}
private class FakeTenant
{
public string Name { get; set; } = "";
}
}
Loading…
Cancel
Save