Address review feedback; filter methods by MethodKind.Ordinary

dotnet · Dec 8, 2020 · 4d23faf · 4d23faf
1 parent 8c56e0d
commit 4d23faf
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 1 deletion.
diff --git a/...wAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs b/...wAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs
@@ -120,6 +120,7 @@ protected override TaintedDataAbstractValue GetDefaultValueForParameterOnEntry(I
                 if (this.DataFlowAnalysisContext.SourceInfos.IsSourceParameter(parameter, WellKnownTypeProvider))
                 {
                     // Location of the parameter, so we can track where the tainted data appears in code.
+                    // The parameter itself may not have any DeclaringSyntaxReferences, e.g. 'value' inside property setters.
                     SyntaxNode parameterSyntaxNode;
                     if (!parameter.DeclaringSyntaxReferences.IsEmpty)
                     {
@@ -131,6 +132,9 @@ protected override TaintedDataAbstractValue GetDefaultValueForParameterOnEntry(I
                     }
                     else
                     {
+                        // Unless there are others, the only case we have for parameters being tainted data sources is inside
+                        // ASP.NET Core MVC controller action methods (see WebInputSources.cs), so those parameters should
+                        // always be declared somewhere.
                         Debug.Fail("Can we have a tainted data parameter with no syntax references?");
                         return ValueDomain.UnknownOrMayBeValue;
                     }

diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/WebInputSources.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/WebInputSources.cs
@@ -89,7 +89,7 @@ static WebInputSources()
                         if (methodSymbol.DeclaredAccessibility != Accessibility.Public
                             || methodSymbol.IsConstructor()
                             || methodSymbol.IsStatic
-                            || methodSymbol.MethodKind == MethodKind.PropertySet
+                            || methodSymbol.MethodKind != MethodKind.Ordinary
                             || methodSymbol.HasDerivedMethodAttribute(wellKnownTypeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftAspNetCoreMvcNonActionAttribute)))
                         {
                             return false;