Skip to content

Commit

Permalink
Fix Returning Correct Number of Results From Conditions (#498)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gfs
gfs authored Aug 11, 2022
1 parent 5e419fe commit 1a083e1
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 51 deletions.
114 changes: 63 additions & 51 deletions AppInspector.RulesEngine/RuleProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,66 +143,78 @@ public List<MatchRecord> AnalyzeFile(TextContainer textContainer, FileEntry file
var caps = _analyzer.GetCaptures(rules, textContainer);
foreach (var ruleCapture in caps)
{
// If we had a WithinClause we only want the captures that passed the within filter.
var filteredCaptures = ruleCapture.Captures.Any(x => x.Clause is WithinClause)
? ruleCapture.Captures.Where(x => x.Clause is WithinClause) : ruleCapture.Captures;
foreach (var cap in filteredCaptures)
List<(int, Boundary)> netCaptures = FilterCaptures(ruleCapture.Captures);
var oatRule = ruleCapture.Rule as ConvertedOatRule;
foreach (var match in netCaptures)
{
resultsList.AddRange(ProcessBoundary(cap));
}
var patternIndex = match.Item1;
var boundary = match.Item2;
//restrict adds from build files to tags with "metadata" only to avoid false feature positives that are not part of executable code
if (!_opts.AllowAllTagsInBuildFiles && languageInfo.Type == LanguageInfo.LangFileType.Build && (oatRule.AppInspectorRule.Tags?.Any(v => !v.Contains("Metadata")) ?? false))
{
continue;
}
if (!_opts.ConfidenceFilter.HasFlag(oatRule.AppInspectorRule.Patterns[patternIndex].Confidence))
{
continue;
}

List<MatchRecord> ProcessBoundary(ClauseCapture cap)
{
List<MatchRecord> newMatches = new();//matches for this rule clause only
Location startLocation = textContainer.GetLocation(boundary.Index);
Location endLocation = textContainer.GetLocation(boundary.Index + boundary.Length);
MatchRecord newMatch = new(oatRule.AppInspectorRule)
{
FileName = fileEntry.FullPath,
FullTextContainer = textContainer,
LanguageInfo = languageInfo,
Boundary = boundary,
StartLocationLine = startLocation.Line,
StartLocationColumn = startLocation.Column,
EndLocationLine = endLocation.Line != 0 ? endLocation.Line : startLocation.Line + 1, //match is on last line
EndLocationColumn = endLocation.Column,
MatchingPattern = oatRule.AppInspectorRule.Patterns[patternIndex],
Excerpt = numLinesContext > 0 ? ExtractExcerpt(textContainer, startLocation.Line, numLinesContext) : string.Empty,
Sample = numLinesContext > -1 ? ExtractTextSample(textContainer.FullContent, boundary.Index, boundary.Length) : string.Empty
};

if (oatRule.AppInspectorRule.Tags?.Contains("Dependency.SourceInclude") ?? false)
{
newMatch.Sample = ExtractDependency(newMatch.FullTextContainer, newMatch.Boundary.Index, newMatch.Pattern, newMatch.LanguageInfo.Name);
}

if (cap is TypedClauseCapture<List<(int, Boundary)>> tcc)
resultsList.Add(newMatch);
}

// If a WithinClause capture is present, use only within captures, otherwise just flattens the list of results from the non-within clause.
List<(int, Boundary)> FilterCaptures(List<ClauseCapture> captures)
{
// If we had a WithinClause we only want the captures that passed the within filter.
if (captures.Any(x => x.Clause is WithinClause))
{
if (ruleCapture.Rule is ConvertedOatRule oatRule)
var onlyWithinCaptures = captures.Where(x => x.Clause is WithinClause).Cast<TypedClauseCapture<List<(int, Boundary)>>>().ToList();
var allCaptured = onlyWithinCaptures.SelectMany(x => x.Result);
ConcurrentDictionary<(int, Boundary), int> numberOfInstances = new();
// If there are multiple within clauses ensure that we only return matches which passed all clauses
// WithinClauses are always ANDed, but each contains all the captures that passed *that* clause.
// We need the captures that passed every clause.
foreach (var aCapture in allCaptured)
{
if (tcc.Result is { } captureResults)
numberOfInstances.AddOrUpdate(aCapture, 1, (tuple, i) => i + 1);
}
return numberOfInstances.Where(x => x.Value == onlyWithinCaptures.Count).Select(x => x.Key).ToList();
}
else
{
var outList = new List<(int, Boundary)>();
foreach (var cap in captures)
{
if (cap is TypedClauseCapture<List<(int, Boundary)>> tcc)
{
foreach (var match in captureResults)
{
var patternIndex = match.Item1;
var boundary = match.Item2;
//restrict adds from build files to tags with "metadata" only to avoid false feature positives that are not part of executable code
if (!_opts.AllowAllTagsInBuildFiles && languageInfo.Type == LanguageInfo.LangFileType.Build && (oatRule.AppInspectorRule.Tags?.Any(v => !v.Contains("Metadata")) ?? false))
{
continue;
}
if (!_opts.ConfidenceFilter.HasFlag(oatRule.AppInspectorRule.Patterns[patternIndex].Confidence))
{
continue;
}

Location startLocation = textContainer.GetLocation(boundary.Index);
Location endLocation = textContainer.GetLocation(boundary.Index + boundary.Length);
MatchRecord newMatch = new(oatRule.AppInspectorRule)
{
FileName = fileEntry.FullPath,
FullTextContainer = textContainer,
LanguageInfo = languageInfo,
Boundary = boundary,
StartLocationLine = startLocation.Line,
StartLocationColumn = startLocation.Column,
EndLocationLine = endLocation.Line != 0 ? endLocation.Line : startLocation.Line + 1, //match is on last line
EndLocationColumn = endLocation.Column,
MatchingPattern = oatRule.AppInspectorRule.Patterns[patternIndex],
Excerpt = numLinesContext > 0 ? ExtractExcerpt(textContainer, startLocation.Line, numLinesContext) : string.Empty,
Sample = numLinesContext > -1 ? ExtractTextSample(textContainer.FullContent, boundary.Index, boundary.Length) : string.Empty
};

if (oatRule.AppInspectorRule.Tags?.Contains("Dependency.SourceInclude") ?? false)
{
newMatch.Sample = ExtractDependency(newMatch.FullTextContainer, newMatch.Boundary.Index, newMatch.Pattern, newMatch.LanguageInfo.Name);
}

newMatches.Add(newMatch);
}
outList.AddRange(tcc.Result);
}
}

return outList;
}
return newMatches;
}
}

Expand Down
100 changes: 100 additions & 0 deletions AppInspector.Tests/RuleProcessor/WithinClauseTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,106 @@ public void WithinClauseValidationTests(bool findingOnlySetting, bool findingReg
Assert.AreEqual(expectedNumIssues, verifier.CheckIntegrity(rules).Sum(x => x.OatIssues.Count()));
}

[TestMethod]
public void WithinClauseWithMultipleConditions()
{
RuleSet rules = new(_loggerFactory);
var newRule = @"[{
""name"": ""Insecure URL"",
""id"": ""DS137138"",
""description"": ""An HTTP-based URL without TLS was detected."",
""recommendation"": ""Update to an HTTPS-based URL if possible."",
""tags"": [
""ThreatModel.Integration.HTTP""
],
""severity"": ""moderate"",
""rule_info"": ""DS137138.md"",
""patterns"": [
{
""pattern"": ""http://"",
""type"": ""substring"",
""scopes"": [
""code""
]
}
],
""conditions"" : [
{
""pattern"" :
{
""pattern"": ""xmlns="",
""type"": ""substring"",
""scopes"": [
""code""
]
},
""negate_finding"": true,
""search_in"": ""finding-region(-1, 0)""
},
{
""pattern"" :
{
""pattern"": ""<!DOCTYPE"",
""type"": ""string"",
""scopes"": [
""code""
]
},
""negate_finding"": true,
""search_in"": ""finding-region(-1, 0)""
},
{
""pattern"" :
{
""pattern"": ""http://localhost"",
""type"": ""regex"",
""scopes"": [
""code""
]
},
""negate_finding"": true,
""search_in"": ""same-line""
},
{
""pattern"" :
{
""pattern"": ""http://127.0.0.1"",
""type"": ""substring"",
""scopes"": [
""code""
]
},
""negate_finding"": true,
""search_in"": ""same-line""
}
],
""must-match"": [
""http://contoso.com""
],
""must-not-match"": [
""http://localhost"",
""http://127.0.0.1"",
""<html xmlns=\""http://www.w3.org/1999/xhtml\"">"",
""<!DOCTYPE html PUBLIC \""-//W3C//DTD XHTML 1.0 Transitional//EN\""\n\""http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\"">""
]
}]";
var testData = @"
http://
http://localhost
xmlns=
http://
http://
";
rules.AddString(newRule, "TestRules");
Microsoft.ApplicationInspector.RulesEngine.RuleProcessor processor = new(rules, new RuleProcessorOptions(){Parallel = false});
if (_languages.FromFileNameOut("test.c", out LanguageInfo info))
{
List<MatchRecord> matches = processor.AnalyzeFile(testData,
new Microsoft.CST.RecursiveExtractor.FileEntry("test.cs", new MemoryStream()), info);
Assert.AreEqual(2, matches.Count);
}
}

[DataRow(true, 1, new[] { 2 })]
[DataRow(false, 1, new[] { 3 })]
[DataTestMethod]
Expand Down

0 comments on commit 1a083e1

Please sign in to comment.