Merge pull request #8626 from michaelnebel/csharp/equalsgethashcodeoverrides

C#: Exclude Equals and GetHashCode overrides from model generation.

csharp/ql/src/utils/model-generator/internal/CaptureModelsSpecific.qll

@@ -6,6 +6,7 @@ private import csharp as CS
 private import semmle.code.csharp.commons.Util as Util
 private import semmle.code.csharp.commons.Collections as Collections
 private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
+private import semmle.code.csharp.frameworks.System as System
 import semmle.code.csharp.dataflow.ExternalFlow as ExternalFlow
 import semmle.code.csharp.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon
 import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate
@@ -16,14 +17,34 @@ module TaintTracking = CS::TaintTracking;
 
 class Type = CS::Type;
 
+/**
+ * Holds if `api` is an override or an interface implementation that
+ * is irrelevant to the data flow analysis.
+ */
+private predicate isIrrelevantOverrideOrImplementation(CS::Callable api) {
+  exists(CS::Callable exclude, CS::Method m |
+    (
+      api = m.getAnOverrider*().getUnboundDeclaration()
+      or
+      api = m.getAnUltimateImplementor().getUnboundDeclaration()
+    ) and
+    exclude = m.getUnboundDeclaration()
+  |
+    exists(System::SystemObjectClass c | exclude = [c.getGetHashCodeMethod(), c.getEqualsMethod()])
+    or
+    exists(System::SystemIEquatableTInterface i | exclude = i.getEqualsMethod())
+  )
+}
+
 /**
  * Holds if it is relevant to generate models for `api`.
  */
 private predicate isRelevantForModels(CS::Callable api) {
   [api.(CS::Modifiable), api.(CS::Accessor).getDeclaration()].isEffectivelyPublic() and
+  api.getDeclaringType().getNamespace().getQualifiedName() != "" and
   not api instanceof CS::ConversionOperator and
   not api instanceof Util::MainMethod and
-  api.getDeclaringType().getNamespace().getQualifiedName() != ""
+  not isIrrelevantOverrideOrImplementation(api)
 }
 
 /**

csharp/ql/test/utils/model-generator/CaptureSummaryModels.expected

@@ -18,6 +18,7 @@
 | Summaries;DerivedClass1Flow;false;ReturnParam1;(System.Int32,System.Int32);Argument[1];ReturnValue;taint |
 | Summaries;DerivedClass2Flow;false;ReturnParam0;(System.Int32,System.Int32);Argument[0];ReturnValue;taint |
 | Summaries;DerivedClass2Flow;false;ReturnParam;(System.Int32);Argument[0];ReturnValue;taint |
+| Summaries;EqualsGetHashCodeNoFlow;false;Equals;(System.Int32);Argument[0];ReturnValue;taint |
 | Summaries;GenericFlow<>;false;AddFieldToGenericList;(System.Collections.Generic.List<T>);Argument[Qualifier];Argument[0].Element;taint |
 | Summaries;GenericFlow<>;false;AddToGenericList<>;(System.Collections.Generic.List<S>,S);Argument[1];Argument[0].Element;taint |
 | Summaries;GenericFlow<>;false;ReturnFieldInGenericList;();Argument[Qualifier];ReturnValue;taint |

csharp/ql/test/utils/model-generator/NoSummaries.cs

@@ -42,4 +42,22 @@ public sealed class PublicClassNoFlow
             }
         }
     }
+}
+
+public class EquatableBound : IEquatable<int>
+{
+    public readonly bool tainted;
+    public bool Equals(int other)
+    {
+        return tainted;
+    }
+}
+
+public class EquatableUnBound<T> : IEquatable<T>
+{
+    public readonly bool tainted;
+    public bool Equals(T? other)
+    {
+        return tainted;
+    }
 }

csharp/ql/test/utils/model-generator/Summaries.cs

@@ -206,4 +206,28 @@ public class OperatorFlow
         return new OperatorFlow(b);
     }
 
+}
+
+public class EqualsGetHashCodeNoFlow
+{
+    public readonly bool boolTainted;
+    public readonly int intTainted;
+
+    // No flow summary as this is an override of the Equals method.
+    public override bool Equals(object obj)
+    {
+        return boolTainted;
+    }
+
+    // Flow summary as this is not an override of the object Equals method.
+    public int Equals(int i)
+    {
+        return i;
+    }
+
+    // No flow summary as this is an override of the GetHashCode method.
+    public override int GetHashCode()
+    {
+        return intTainted;
+    }
 }