Skip to content

Commit 36e2c5a

Browse files
erlendellefsenerlendellefsen
authored
feat: add recursion depth guard for nested collection filters (#66)
- Add MaxRecursionDepth (5) to prevent stack overflow from malicious filter paths - Protects against deeply nested collection navigations like items[].items[].items[]... - Returns 400 Bad Request with helpful error message when limit exceeded
1 parent df411fe commit 36e2c5a

2 files changed

Lines changed: 205 additions & 4 deletions

File tree

‎JsonApiToolkit.Tests/Extensions/RecursionDepthGuardTests.cs‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
using System.Linq.Expressions;
2+
using JsonApiToolkit.Extensions.Querying;
3+
using JsonApiToolkit.Models.Errors;
4+
using JsonApiToolkit.Models.Querying.Filtering;
5+
6+
namespace JsonApiToolkit.Tests.Extensions;
7+
8+
public class RecursionDepthGuardTests
9+
{
10+
// Test entity with nested collections to trigger recursion
11+
private class Level0
12+
{
13+
public int Id { get; set; }
14+
public List<Level1> Items { get; set; } = [];
15+
}
16+
17+
private class Level1
18+
{
19+
public int Id { get; set; }
20+
public List<Level2> Items { get; set; } = [];
21+
}
22+
23+
private class Level2
24+
{
25+
public int Id { get; set; }
26+
public List<Level3> Items { get; set; } = [];
27+
}
28+
29+
private class Level3
30+
{
31+
public int Id { get; set; }
32+
public List<Level4> Items { get; set; } = [];
33+
}
34+
35+
private class Level4
36+
{
37+
public int Id { get; set; }
38+
public List<Level5> Items { get; set; } = [];
39+
}
40+
41+
private class Level5
42+
{
43+
public int Id { get; set; }
44+
public List<Level6> Items { get; set; } = [];
45+
}
46+
47+
private class Level6
48+
{
49+
public int Id { get; set; }
50+
public string Name { get; set; } = "";
51+
}
52+
53+
[Fact]
54+
public void BuildFilterExpression_WithShallowNesting_Succeeds()
55+
{
56+
// 2 levels of collection nesting should work fine
57+
var filter = new FilterParameter
58+
{
59+
Field = "items.items.id",
60+
Value = "1",
61+
Operator = FilterOperator.Eq,
62+
};
63+
64+
var parameter = Expression.Parameter(typeof(Level0), "x");
65+
66+
// This should not throw
67+
var expression = NestedPropertyNavigator.BuildSafeNestedFilterExpression(parameter, filter);
68+
69+
Assert.NotNull(expression);
70+
}
71+
72+
[Fact]
73+
public void BuildFilterExpression_WithDeeplyNestedCollections_ThrowsBadRequest()
74+
{
75+
// 7 levels of collection nesting should exceed the limit (MaxRecursionDepth = 5)
76+
var filter = new FilterParameter
77+
{
78+
Field = "items.items.items.items.items.items.name",
79+
Value = "test",
80+
Operator = FilterOperator.Eq,
81+
};
82+
83+
var parameter = Expression.Parameter(typeof(Level0), "x");
84+
85+
var exception = Assert.Throws<JsonApiBadRequestException>(() =>
86+
NestedPropertyNavigator.BuildSafeNestedFilterExpression(parameter, filter)
87+
);
88+
89+
Assert.Contains("recursion depth", exception.Message.ToLower());
90+
Assert.Contains("5", exception.Message); // MaxRecursionDepth
91+
Assert.Equal(JsonApiErrorCodes.QueryTooComplex, exception.Code);
92+
}
93+
94+
[Fact]
95+
public void BuildFilterExpression_AtExactLimit_Succeeds()
96+
{
97+
// 5 levels should be exactly at the limit and work
98+
var filter = new FilterParameter
99+
{
100+
Field = "items.items.items.items.items.id",
101+
Value = "1",
102+
Operator = FilterOperator.Eq,
103+
};
104+
105+
var parameter = Expression.Parameter(typeof(Level0), "x");
106+
107+
// This should not throw - exactly at limit
108+
var expression = NestedPropertyNavigator.BuildSafeNestedFilterExpression(parameter, filter);
109+
110+
Assert.NotNull(expression);
111+
}
112+
113+
[Fact]
114+
public void BuildFilterExpression_JustOverLimit_ThrowsBadRequest()
115+
{
116+
// 6 levels should be just over the limit
117+
var filter = new FilterParameter
118+
{
119+
Field = "items.items.items.items.items.items.id",
120+
Value = "1",
121+
Operator = FilterOperator.Eq,
122+
};
123+
124+
var parameter = Expression.Parameter(typeof(Level0), "x");
125+
126+
var exception = Assert.Throws<JsonApiBadRequestException>(() =>
127+
NestedPropertyNavigator.BuildSafeNestedFilterExpression(parameter, filter)
128+
);
129+
130+
Assert.Contains("recursion depth", exception.Message.ToLower());
131+
}
132+
133+
[Fact]
134+
public void BuildFilterExpression_ErrorMetadata_ContainsFieldInfo()
135+
{
136+
var filter = new FilterParameter
137+
{
138+
Field = "items.items.items.items.items.items.name",
139+
Value = "test",
140+
Operator = FilterOperator.Eq,
141+
};
142+
143+
var parameter = Expression.Parameter(typeof(Level0), "x");
144+
145+
var exception = Assert.Throws<JsonApiBadRequestException>(() =>
146+
NestedPropertyNavigator.BuildSafeNestedFilterExpression(parameter, filter)
147+
);
148+
149+
Assert.NotNull(exception.ErrorSource);
150+
Assert.StartsWith("filter[", exception.ErrorSource.Parameter);
151+
Assert.NotNull(exception.Meta);
152+
Assert.Equal(5, exception.Meta["maxDepth"]);
153+
Assert.True((int)exception.Meta["actualDepth"] > 5); // Should exceed the limit
154+
}
155+
}

‎JsonApiToolkit/Extensions/Querying/Filtering/NestedPropertyNavigator.cs‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Reflection;
33
using System.Text.RegularExpressions;
44
using JsonApiToolkit.Helpers;
5+
using JsonApiToolkit.Models.Errors;
56
using JsonApiToolkit.Models.Querying.Filtering;
67
using Microsoft.Extensions.Logging;
78

@@ -11,6 +12,12 @@ internal static partial class NestedPropertyNavigator
1112
{
1213
private const int MaxLogValueLength = 100;
1314

15+
/// <summary>
16+
/// Maximum recursion depth for nested collection navigations.
17+
/// Prevents stack overflow from malicious deeply nested filter paths.
18+
/// </summary>
19+
private const int MaxRecursionDepth = 5;
20+
1421
/// <summary>
1522
/// Sanitizes user input for safe logging by removing control characters
1623
/// and truncating long values to prevent log forging attacks.
@@ -36,9 +43,26 @@ private static string SanitizeForLog(string? value)
3643
internal static Expression? BuildSafeNestedFilterExpression(
3744
ParameterExpression parameter,
3845
FilterParameter filter,
39-
ILogger? logger = null
46+
ILogger? logger = null,
47+
int depth = 0
4048
)
4149
{
50+
if (depth > MaxRecursionDepth)
51+
{
52+
throw new JsonApiBadRequestException(
53+
$"Filter path recursion depth exceeds maximum of {MaxRecursionDepth}. "
54+
+ "Simplify the filter expression or reduce collection nesting.",
55+
JsonApiErrorCodes.QueryTooComplex,
56+
new ErrorSource { Parameter = $"filter[{filter.Field}]" },
57+
new Dictionary<string, object>
58+
{
59+
["field"] = filter.Field,
60+
["maxDepth"] = MaxRecursionDepth,
61+
["actualDepth"] = depth,
62+
}
63+
);
64+
}
65+
4266
string[] parts = filter.Field.Split('.');
4367
Expression current = parameter;
4468
var nullChecks = new List<Expression>();
@@ -69,7 +93,8 @@ private static string SanitizeForLog(string? value)
6993
elementType,
7094
remainingParts,
7195
filter,
72-
logger
96+
logger,
97+
depth + 1
7398
);
7499

75100
if (collectionFilter == null)
@@ -174,9 +199,25 @@ private static string SanitizeForLog(string? value)
174199
Type elementType,
175200
string[] remainingParts,
176201
FilterParameter filter,
177-
ILogger? logger
202+
ILogger? logger,
203+
int depth
178204
)
179205
{
206+
if (depth > MaxRecursionDepth)
207+
{
208+
throw new JsonApiBadRequestException(
209+
$"Filter path recursion depth exceeds maximum of {MaxRecursionDepth}. "
210+
+ "Simplify the filter expression or reduce collection nesting.",
211+
JsonApiErrorCodes.QueryTooComplex,
212+
new ErrorSource { Parameter = $"filter[{filter.Field}]" },
213+
new Dictionary<string, object>
214+
{
215+
["field"] = filter.Field,
216+
["maxDepth"] = MaxRecursionDepth,
217+
["actualDepth"] = depth,
218+
}
219+
);
220+
}
180221
// Create parameter for the lambda: item =>
181222
ParameterExpression itemParam = Expression.Parameter(elementType, "item");
182223

@@ -210,7 +251,12 @@ private static string SanitizeForLog(string? value)
210251
else
211252
{
212253
// Nested property access - recursively build
213-
innerExpression = BuildSafeNestedFilterExpression(itemParam, innerFilter, logger);
254+
innerExpression = BuildSafeNestedFilterExpression(
255+
itemParam,
256+
innerFilter,
257+
logger,
258+
depth
259+
);
214260
}
215261

216262
if (innerExpression == null)

0 commit comments

Comments
 (0)