Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ namespace Microsoft.SCIM
// and the second pair of bracketed clauses are in a third group.
internal sealed class FilterExpression : IFilterExpression
{
// Defense-in-depth caps for the recursive FilterExpression parser. Without these caps an
// attacker can submit a filter string with thousands of clauses chained by 'and'/'or' and
// trigger a StackOverflowException, which is non-catchable in .NET and terminates the
// process. Real SCIM filters are <2KB and have <20 clauses; these caps leave generous
// headroom while bounding worst-case recursion depth.
private const int MaxFilterLengthChars = 16_384;
private const int MaxRecursionDepth = 32;

private const char BracketClose = ')';
private const char Escape = '\\';
private const char Quote = '"';
Expand Down Expand Up @@ -99,6 +107,7 @@ internal sealed class FilterExpression : IFilterExpression
private ComparisonOperator filterOperator;
Comment thread
himanshusainig marked this conversation as resolved.
private int groupValue;
private int levelValue;
private int depth;
Comment thread
himanshusainig marked this conversation as resolved.
private LogicalOperatorValue logicalOperator;
private FilterExpression next;
private ComparisonValue value;
Expand All @@ -116,6 +125,7 @@ private FilterExpression(FilterExpression other)
this.filterOperator = other.filterOperator;
this.Group = other.Group;
this.Level = other.Level;
this.depth = other.depth;
this.logicalOperator = other.logicalOperator;
this.value = other.value;
if (other.next != null)
Expand All @@ -126,17 +136,49 @@ private FilterExpression(FilterExpression other)
}

private FilterExpression(string text, int group, int level)
: this(text, group, level, depth: 0)
{
}

private FilterExpression(string text, int group, int level, int depth)
{
if (string.IsNullOrWhiteSpace(text))
{
throw new ArgumentNullException(nameof(text));
}

// Top-level only: reject overlong inputs before any parsing begins.
if (depth == 0 && text.Length > MaxFilterLengthChars)
{
throw new ArgumentException(
string.Format(
CultureInfo.InvariantCulture,
"Filter expression length {0} exceeds maximum allowed length {1}.",
text.Length,
MaxFilterLengthChars),
nameof(text));
}

// Per-recursion: reject if we've exceeded the recursion-depth budget. The parser
// recurses once per logical operator ('and'/'or'); this depth cap bounds worst-case
// recursion even if the input length cap is bypassed by future callers.
if (depth >= MaxRecursionDepth)
{
throw new ArgumentException(
string.Format(
CultureInfo.InvariantCulture,
"Filter expression nesting exceeds maximum allowed depth {0}.",
MaxRecursionDepth),
nameof(text));
}

this.Text = text.Trim();

this.Level = level;
this.Group = group;

this.depth = depth;

MatchCollection matches = FilterExpression.Expression.Value.Matches(this.Text);
foreach (Match match in matches)
{
Expand Down Expand Up @@ -627,7 +669,7 @@ private void Initialize(Group left, Group @operator, Group right)
nextExpressionLevel = this.Level;
nextExpressionGroup = this.Group;
}
this.next = new FilterExpression(nextExpression, nextExpressionGroup, nextExpressionLevel);
this.next = new FilterExpression(nextExpression, nextExpressionGroup, nextExpressionLevel, this.depth + 1);
this.next.Previous = this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ public sealed class Path : IPath
{
private const string ArgumentNamePathExpression = "pathExpression";

// Defense-in-depth caps for the recursive Path.TryParse parser. Without these caps an
// attacker can submit a path with thousands of dot-separated segments and trigger a
// StackOverflowException, which is non-catchable in .NET and terminates the process.
// Real SCIM paths are <200 chars and have <5 dot-separators; these caps leave generous
// headroom while bounding worst-case recursion depth.
private const int MaxPathLengthChars = 16_384;
private const int MaxRecursionDepth = 32;

private const string ConstructNameSubAttributes = "subAttr";
private const string ConstructNameValuePath = "valuePath";
private const string PatternTemplate = @"(?<{0}>.*)\[(?<{1}>.*)\]";
Expand Down Expand Up @@ -139,6 +147,11 @@ private static bool TryExtractSchemaIdentifier(string pathExpression, out string
}

public static bool TryParse(string pathExpression, out IPath path)
{
return TryParse(pathExpression, depth: 0, out path);
}

private static bool TryParse(string pathExpression, int depth, out IPath path)
{
path = null;

Expand All @@ -147,6 +160,20 @@ public static bool TryParse(string pathExpression, out IPath path)
throw new ArgumentNullException(Path.ArgumentNamePathExpression);
}

// Top-level only: reject overlong inputs before any parsing begins.
if (depth == 0 && pathExpression.Length > MaxPathLengthChars)
{
return false;
}

// Per-recursion: reject if we've exceeded the recursion-depth budget. The parser
// recurses once per bracket filter expression (valuePathExpression inside [...]);
// this depth cap bounds worst-case recursion even if the input length cap is bypassed.
if (depth >= MaxRecursionDepth)
{
return false;
}

Path buffer = new Path(pathExpression);

string expression = pathExpression;
Expand All @@ -164,7 +191,7 @@ public static bool TryParse(string pathExpression, out IPath path)

expression = expression.Substring(0, seperatorIndex);

if (!Path.TryParse(valuePathExpression, out IPath valuePath))
if (!Path.TryParse(valuePathExpression, depth + 1, out IPath valuePath))
{
return false;
}
Expand Down