Merge pull request diffblue#333 from diffblue/bugfix/more_input_params_per_rule

SEC-222: Extended taint rules so that there can be more than one rule for one function.

Author: marek-trtik

regression/end_to_end/multitaint/rules.json

@@ -12,7 +12,7 @@
       }
     },
     {
-      "comment": "Put tainted name to cookie",
+      "comment": "Put tainted name to cookie via the first parameter.",
       "class": "Cookie",
       "method": "<init>:(Ljava/lang/Object;Ljava/lang/Object;)V",
       "input": {
@@ -24,6 +24,19 @@
         "taint": "Tainted cookie"
       }
     },
+    {
+      "comment": "Put tainted name to cookie via the second parameter.",
+      "class": "Cookie",
+      "method": "<init>:(Ljava/lang/Object;Ljava/lang/Object;)V",
+      "input": {
+        "location": "arg2",
+        "taint": "Tainted data"
+      },
+      "result": {
+        "location": "this",
+        "taint": "Tainted cookie"
+      }
+    },
     {
       "comment": "Writing potentially tainted data to a sink.",
       "class": "Main",

regression/end_to_end/multitaint/src/Main.java

@@ -21,4 +21,13 @@ public static void taint_via_value() {
         Cookie c = new Cookie(name, value);
         sink(c);
     }
+
+    public static void taint_via_both() {
+        Object name = new Object();
+        Object value = new Object();
+        makeTainted(name);
+        makeTainted(value);
+        Cookie c = new Cookie(name, value);
+        sink(c);
+    }
 }

regression/end_to_end/multitaint/test_multitaint.py

@@ -1,24 +1,17 @@
 import regression.end_to_end.driver as pipeline_executor
 import os
 import subprocess
-import pytest
 import regression.utils as utils
 
 
-@pytest.mark.xfail(strict=True)
 def test_multitaint():
-    """
-    The test case shows a weakness in our rules system. The test defines
-    a class Cookie, whose constructor takes 2 arguments, and each of them
-    can bring tainted data to the created instance. However, our rule
-    allows only for 1 tainted input.
-    """
     with utils.working_dir(os.path.abspath(os.path.dirname(__file__))):
         subprocess.call("ant")
     traces = pipeline_executor.run_security_analyser_pipeline(
         os.path.join("dist", "multitaint.jar"),
         "rules.json",
         os.path.realpath(os.path.dirname(__file__)))
-    assert traces.count_traces() == 2
-    assert traces.trace_exists("java::Main.taint_via_name:()V", 13)
-    assert traces.trace_exists("java::Main.taint_via_value:()V", 20)
+    assert traces.count_traces() == 3
+    assert traces.trace_exists("java::Main.taint_via_name:()V", 14)
+    assert traces.trace_exists("java::Main.taint_via_value:()V", 22)
+    assert traces.trace_exists("java::Main.taint_via_both:()V", 31)

src/taint-analysis/taint_rules.h

@@ -14,6 +14,7 @@
 #include <util/message.h>
 #include <map>
 #include <string>
+#include <vector>
 #include <memory>
 #include <boost/optional.hpp>
 
@@ -196,9 +197,9 @@ class taint_sink_rulet : public taint_rulet
 class taint_rulest
 {
 private:
-  std::map<irep_idt, taint_propagation_rulet> propagation_rules;
-  std::map<irep_idt, taint_sanitizer_rulet> sanitizer_rules;
-  std::map<irep_idt, taint_sink_rulet> sink_rules;
+  std::multimap<irep_idt, taint_propagation_rulet> propagation_rules;
+  std::multimap<irep_idt, taint_sanitizer_rulet> sanitizer_rules;
+  std::multimap<irep_idt, taint_sink_rulet> sink_rules;
 
   taint_rulest() { }
 
@@ -233,41 +234,49 @@ class taint_rulest
       || sink_rules.find(fn_name) != sink_rules.end();
   }
 
-  boost::optional<const taint_propagation_rulet &> find_propagation_rule(
-    const irep_idt &fn_name) const
+  std::vector<std::reference_wrapper<const taint_propagation_rulet>>
+  find_propagation_rule(const irep_idt &fn_name) const
   {
     // We currently match directly on the name
     // TODO: Search for any methods derived on base classes of the class name
     //  used in the call
-    // class_hierarchyt::idst parents = class_hierarchy.get_parents_trans(class_id);
-    auto it = propagation_rules.find(fn_name);
-    if(it == propagation_rules.end())
-      return boost::none;
-    return it->second;
+    std::vector<std::reference_wrapper<const taint_propagation_rulet>>
+      matching_rules;
+    auto range = propagation_rules.equal_range(fn_name);
+    for(auto it = range.first; it != range.second; ++it)
+      matching_rules.push_back(
+        std::reference_wrapper<const taint_propagation_rulet>(it->second));
+    return matching_rules;
   }
 
-  boost::optional<const taint_sanitizer_rulet &> find_sanitizer_rule(const irep_idt &fn_name) const
+  std::vector<std::reference_wrapper<const taint_sanitizer_rulet>>
+  find_sanitizer_rule(const irep_idt &fn_name) const
   {
     // We currently match directly on the name
     // TODO: Search for any methods derived on base classes of the class name
     //  used in the call
-    // class_hierarchyt::idst parents = class_hierarchy.get_parents_trans(class_id);
-    auto it = sanitizer_rules.find(fn_name);
-    if(it == sanitizer_rules.end())
-      return boost::none;
-    return it->second;
+    std::vector<std::reference_wrapper<const taint_sanitizer_rulet>>
+      matching_rules;
+    auto range = sanitizer_rules.equal_range(fn_name);
+    for(auto it = range.first; it != range.second; ++it)
+      matching_rules.push_back(
+        std::reference_wrapper<const taint_sanitizer_rulet>(it->second));
+    return matching_rules;
   }
 
-  boost::optional<const taint_sink_rulet &> find_sink_rule(const irep_idt &fn_name) const
+  std::vector<std::reference_wrapper<const taint_sink_rulet>>
+  find_sink_rule(const irep_idt &fn_name) const
   {
     // We currently match directly on the name
     // TODO: Search for any methods derived on base classes of the class name
     //  used in the call
     // class_hierarchyt::idst parents = class_hierarchy.get_parents_trans(class_id);
-    auto it = sink_rules.find(fn_name);
-    if(it == sink_rules.end())
-      return boost::none;
-    return it->second;
+    std::vector<std::reference_wrapper<const taint_sink_rulet>> matching_rules;
+    auto range = sink_rules.equal_range(fn_name);
+    for(auto it = range.first; it != range.second; ++it)
+      matching_rules.push_back(
+        std::reference_wrapper<const taint_sink_rulet>(it->second));
+    return matching_rules;
   }