Merge pull request #1021 from calumgrant/cs/nhibernate

C#: Model NHibernate framework

change-notes/1.20/analysis-csharp.md

@@ -34,5 +34,9 @@
   - Stored data flow sources
   - Sinks for SQL expressions
   - Data flow through fields that are mapped to the database.
+* Support has been added for NHibernate-Core, including
+  - Stored data flow sources
+  - Sinks for SQL expressions
+  - Data flow through fields that are mapped to the database.  
 
 ## Changes to the autobuilder

csharp/ql/src/semmle/code/csharp/dataflow/DataFlow.qll

@@ -10,6 +10,7 @@ module DataFlow {
   private import semmle.code.csharp.dataflow.DelegateDataFlow
   private import semmle.code.csharp.dataflow.LibraryTypeDataFlow
   private import semmle.code.csharp.frameworks.EntityFramework
+  private import semmle.code.csharp.frameworks.NHibernate
   private import Internal::Cached
   private import dotnet
   private import cil

csharp/ql/src/semmle/code/csharp/frameworks/NHibernate.qll

@@ -0,0 +1,113 @@
+import csharp
+private import semmle.code.csharp.frameworks.System
+private import semmle.code.csharp.frameworks.system.Collections
+private import semmle.code.csharp.frameworks.Sql
+
+module NHibernate {
+  /** A class that is mapped to the database. */
+  abstract class MappedClass extends Class { }
+
+  /** The interface `NHibernamte.ISession`. */
+  class ISessionInterface extends Interface {
+    ISessionInterface() { this.hasQualifiedName("NHibernate.ISession") }
+
+    /** Gets a parameter that uses a mapped object. */
+    Parameter getAMappedObjectParameter() {
+      exists(Callable c |
+        result.getType() instanceof ObjectType and
+        c = this.getAMethod() and
+        result = c.getAParameter() and
+        result.getName() = "obj"
+      )
+    }
+
+    /** Gets a type parameter that specifies a mapped class. */
+    TypeParameter getAMappedObjectTp() {
+      exists(string methodName |
+        methodName = "Load"
+        or
+        methodName = "Merge"
+        or
+        methodName = "Get"
+        or
+        methodName = "Query"
+      |
+        result = this.getAMethod(methodName).(UnboundGenericMethod).getTypeParameter(0)
+      )
+    }
+  }
+
+  /** A mapped class that is mapped because it is used as a type argument. */
+  private class MappedByTypeArgument extends MappedClass {
+    MappedByTypeArgument() {
+      this = any(ISessionInterface si).getAMappedObjectTp().getASuppliedType()
+    }
+  }
+
+  /** A mapped class that is mapped because it is passed as a parameter. */
+  private class MappedByParam extends MappedClass {
+    MappedByParam() {
+      exists(ISessionInterface si, Expr e, MethodCall c, Parameter p |
+        p = si.getAMappedObjectParameter() and
+        e = c.getArgumentForParameter(p) and
+        this = e.getType()
+      ) and
+      not this instanceof ObjectType and
+      not this.getABaseInterface*() instanceof SystemCollectionsIEnumerableInterface and
+      not this instanceof SystemTypeClass
+    }
+  }
+
+  /** A property that is persisted in the database. */
+  class MappedProperty extends Property {
+    MappedProperty() {
+      this.getDeclaringType() instanceof MappedClass and
+      this.isPublic()
+    }
+  }
+
+  /** A parameter that is interpreted as SQL. */
+  class SqlParameter extends Parameter {
+    SqlParameter() {
+      this.getType() instanceof StringType and
+      (this.getName() = "sql" or this.getName() = "sqlString" or this.getName() = "query") and
+      this
+          .getCallable()
+          .getDeclaringType()
+          .getDeclaringNamespace()
+          .getParent*()
+          .hasQualifiedName("", "NHibernate")
+    }
+  }
+
+  /** A call to a method in NHibernate that executes SQL. */
+  class NHibernateSqlSink extends SqlExpr, Call {
+    SqlParameter sqlParam;
+
+    NHibernateSqlSink() { this.getTarget().getAParameter() = sqlParam }
+
+    override Expr getSql() { result = this.getArgumentForParameter(sqlParam) }
+  }
+
+  /** A taint source where the data has come from a mapped property stored in the database. */
+  class StoredFlowSource extends DataFlow::Node {
+    StoredFlowSource() {
+      this.asExpr() = any(PropertyRead read | read.getTarget() instanceof MappedProperty)
+    }
+  }
+
+  /**
+   * A dataflow node whereby data flows from a property write to a property read
+   * via some database. The assumption is that all writes can flow to all reads.
+   */
+  class MappedPropertyJumpNode extends DataFlow::NonLocalJumpNode {
+    MappedProperty property;
+
+    MappedPropertyJumpNode() { this.asExpr() = property.getAnAssignedValue() }
+
+    override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {
+      result.asExpr().(PropertyRead).getTarget() = property and
+      preservesValue = false
+    }
+  }
+}

csharp/ql/src/semmle/code/csharp/frameworks/Sql.qll

@@ -4,6 +4,7 @@ import csharp
 private import semmle.code.csharp.frameworks.system.Data
 private import semmle.code.csharp.frameworks.system.data.SqlClient
 private import semmle.code.csharp.frameworks.EntityFramework
+private import semmle.code.csharp.frameworks.NHibernate
 
 /** An expression containing a SQL command. */
 abstract class SqlExpr extends Expr {

csharp/ql/src/semmle/code/csharp/security/dataflow/flowsources/Stored.qll

@@ -6,6 +6,7 @@ import csharp
 private import semmle.code.csharp.frameworks.system.data.Common
 private import semmle.code.csharp.frameworks.system.data.Entity
 private import semmle.code.csharp.frameworks.EntityFramework
+private import semmle.code.csharp.frameworks.NHibernate
 private import semmle.code.csharp.frameworks.Sql
 
 /** A data flow source of stored user input. */
@@ -48,6 +49,9 @@ class DbDataReaderPropertyStoredFlowSource extends StoredFlowSource {
 }
 
 /** A read of a mapped property. */
-class EntityFrameworkMappedProperty extends StoredFlowSource {
-  EntityFrameworkMappedProperty() { this instanceof EntityFramework::StoredFlowSource }
+class ORMMappedProperty extends StoredFlowSource {
+  ORMMappedProperty() {
+    this instanceof EntityFramework::StoredFlowSource or
+    this instanceof NHibernate::StoredFlowSource
+  }
 }

csharp/ql/test/library-tests/frameworks/NHibernate/DataFlow.expected

@@ -0,0 +1,2 @@
+| nhibernate.cs:50:14:50:19 | access to property Name | Data flow from $@. | nhibernate.cs:45:24:45:32 | "tainted" | "tainted" |
+| nhibernate.cs:55:14:55:23 | access to property Address | Data flow from $@. | nhibernate.cs:45:24:45:32 | "tainted" | "tainted" |

csharp/ql/test/library-tests/frameworks/NHibernate/DataFlow.ql

@@ -0,0 +1,18 @@
+import csharp
+import semmle.code.csharp.dataflow.TaintTracking
+
+class MyConfiguration extends TaintTracking::Configuration {
+  MyConfiguration() { this = "MyConfiguration" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.asExpr().(StringLiteral).getValue() = "tainted"
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(MethodCall mc | mc.getTarget().hasName("Sink") and node.asExpr() = mc.getArgument(0))
+  }
+}
+
+from MyConfiguration config, DataFlow::Node source, DataFlow::Node sink
+where config.hasFlow(source, sink)
+select sink, "Data flow from $@.", source, source.toString()

csharp/ql/test/library-tests/frameworks/NHibernate/SqlExprs.expected

@@ -0,0 +1,2 @@
+| nhibernate.cs:16:9:16:26 | object creation of type SqlString |
+| nhibernate.cs:17:9:17:27 | call to method Delete |

csharp/ql/test/library-tests/frameworks/NHibernate/SqlExprs.ql

@@ -0,0 +1,5 @@
+import csharp
+import semmle.code.csharp.frameworks.Sql
+
+from SqlExpr e
+select e

csharp/ql/test/library-tests/frameworks/NHibernate/StoredFlowSources.expected

@@ -0,0 +1,6 @@
+| nhibernate.cs:49:14:49:17 | access to property Id |
+| nhibernate.cs:50:14:50:19 | access to property Name |
+| nhibernate.cs:51:14:51:22 | access to property Address |
+| nhibernate.cs:53:14:53:18 | access to property Id |
+| nhibernate.cs:54:14:54:19 | access to property Age |
+| nhibernate.cs:55:14:55:23 | access to property Address |

csharp/ql/test/library-tests/frameworks/NHibernate/StoredFlowSources.ql

@@ -0,0 +1,5 @@
+import csharp
+import semmle.code.csharp.security.dataflow.flowsources.Stored
+
+from StoredFlowSource source
+select source

csharp/ql/test/library-tests/frameworks/NHibernate/nhibernate.cs

@@ -0,0 +1,62 @@
+// semmle-extractor-options: /r:System.Data.dll /r:System.ComponentModel.Primitives.dll  ${testdir}/../../../resources/stubs/NHibernate.cs ${testdir}/../../../resources/stubs/System.Data.cs /r:System.ComponentModel.TypeConverter.dll /r:System.Data.Common.dll
+
+
+using NHibernate;
+using NHibernate.SqlCommand;
+
+namespace NHibernateTest
+{
+  class Test
+  {
+    ISession session;
+
+    void SqlExprs()
+    {
+        var sql = "sql";
+        new SqlString(sql); // SQL expression
+        session.Delete(sql);  // SQL expression
+    }
+
+    class Person
+    {
+        public int Id { get; set; }
+        public string Name { get; set; }
+        public string Address { get; set; }
+    }
+
+    class Person2
+    {
+        public int Id { get; set; }
+        public int Age { get; set; }
+        public string Address { get; set; }
+    }
+
+    void FlowSources()
+    {
+        session.Query<Person>();
+        session.Save(new Person2());
+    }
+
+    void DataFlow()
+    {
+        var p = new Person();
+        var p2 = new Person2();
+        
+        string taint = "tainted";
+        p.Name = taint;
+        p2.Address = taint;
+
+        Sink(p.Id);  // Not tainted
+        Sink(p.Name); // Tainted
+        Sink(p.Address);  // Not tainted
+        
+        Sink(p2.Id); // Not tainted
+        Sink(p2.Age);  // Not tainted
+        Sink(p2.Address);  // Tainted
+    }
+
+    void Sink(object sink)
+    {
+    }
+  }
+}

csharp/ql/test/resources/stubs/NHibernate.cs

@@ -0,0 +1,18 @@
+
+namespace NHibernate
+{
+    public interface ISession
+    {
+        void Delete(string query);
+        T Query<T>();
+        void Save(object obj);
+    }
+
+    namespace SqlCommand
+    {
+        public class SqlString
+        {
+            public SqlString(string sql) { }
+        }
+    }
+}