Merge pull request diffblue#385 from diffblue/enabling_reachability_slicer_instead_of_full_one

[SEC-354] Switch default slicer in the pipeline from full- to reachability-slicer

Author: NathanJPhillips

driver/analyser.py

@@ -127,8 +127,8 @@ def run_program_slicing(
 
         command = (
             get_goto_instrument_pathname() + " " +
-            # "--reachability-slice " +
-            "--full-slice " +
+            "--reachability-slice " +
+            # "--full-slice " +
             "--verbosity " + str(verbosity) + " " +
             cfg["goto_binary_file"] + " " +
             dst_goto_program_fname + " " +

regression/end_to_end/driver.py

@@ -19,7 +19,6 @@ def count_traces(self):
         for trace in self.traces:
             for prog_traces in trace["error_traces"].values():
                 count += len(prog_traces)
-
         return count
 
     def trace_exists(self, function, line_no):
@@ -36,6 +35,14 @@ def trace_of_length_exists(self, function, line_no, tool_name, trace_length):
                         return True
         return False
 
+    def count_specific_traces(self, function, line_no, tool_name):
+        count = 0
+        for trace in self.traces:
+            if trace["function"] == function and trace["line"] == line_no:
+                for error_trace in trace["error_traces"][tool_name]:
+                    count += 1
+        return count
+
     def trace_goes_through(
             self,
             tool_name,

regression/end_to_end/taint_over_list/test_taint_over_list.py

@@ -1,17 +1,13 @@
 import os
-import pytest
 import subprocess
 
 from regression.end_to_end.driver import run_security_analyser_pipeline
 import regression.utils as utils
 
 
-@pytest.mark.xfail(strict=True)
 def test_taint_over_list():
-    """
-    The problem is in the full-slicer. It removes code from ArrayList.add.
-    When the slicer is disabled (skipped) the results is correct.
-    """
+    # This test fails with the full-slicer on because it removes code from
+    # ArrayList.add.
     with utils.working_dir(os.path.abspath(os.path.dirname(__file__))):
         subprocess.call(["flock", ".build_lock", "ant"])
     traces = run_security_analyser_pipeline(

regression/end_to_end/taint_over_list_models/test_taint_over_list_models.py

@@ -6,14 +6,10 @@
 import regression.utils as utils
 
 
-@pytest.mark.xfail(strict=True)
+# This test also crashes with the full-slicer on
+@pytest.mark.xfail("SECURITY_REGRESSION_TESTS_USE_CSVSA" in os.environ,
+                   strict=True, reason="Zero traces found with CSVSA")
 def test_taint_over_list_models():
-    """
-    The test crashes in the taint analysis:
-        security-scanner/src/taint-analysis/taint_summary.cpp:663:
-            Assertion `replace_it!=program->get_NONDET_retvals_replacements().cend()'
-            failed.
-    """
     with utils.working_dir(os.path.abspath(os.path.dirname(__file__))):
         subprocess.call(["flock", ".build_lock", "ant"])
     traces = run_security_analyser_pipeline(

regression/end_to_end/taint_two_tokens/rules.json

@@ -12,7 +12,7 @@
         }
       },
       {
-        "comment": "Read from tainted stream gives tainted string and tainted bytes.",
+        "comment": "Read from tainted stream taints passed bytes.",
         "class": "two.tokens.ServletInputStream",
         "method": "read:([B)Ltwo/tokens/String;",
         "input": {
@@ -26,7 +26,7 @@
         }
       },
       {
-        "comment": "Read from tainted stream gives tainted string and tainted bytes.",
+        "comment": "Read from tainted stream gives tainted string.",
         "class": "two.tokens.ServletInputStream",
         "method": "read:([B)Ltwo/tokens/String;",
         "input": {

regression/end_to_end/taint_two_tokens/test_taint_two_tokens.py

@@ -6,7 +6,6 @@
 
 
 def test_taint_two_tokens():
-
     with utils.working_dir(os.path.abspath(os.path.dirname(__file__))):
         subprocess.call(["flock", ".build_lock", "ant"])
     with run_security_analyser_pipeline(
@@ -15,16 +14,11 @@ def test_taint_two_tokens():
             os.path.realpath(os.path.dirname(__file__)),
             "two.tokens.Main.doGet") as traces:
 
-        # There are two traces here but they are both linked to the same line number even if the goto assert operation
-        # is different. Right now we can't tell the difference between the two, except of their length.
-        assert traces.count_traces() == 2
-        assert traces.trace_of_length_exists(
+        # There should be two traces linked to the same line number generated
+        #  from two different goto assert operations.
+        assert traces.count_specific_traces(
             "java::two.tokens.Main.doGet:(Ltwo/tokens/HttpServletRequest;Ltwo/tokens/HttpServletResponse;)V",
             28,
-            "jbmc",
-            66 if "SECURITY_REGRESSION_TESTS_USE_CSVSA" in os.environ else 63)
-        assert traces.trace_of_length_exists(
-            "java::two.tokens.Main.doGet:(Ltwo/tokens/HttpServletRequest;Ltwo/tokens/HttpServletResponse;)V",
-            28,
-            "jbmc",
-            68 if "SECURITY_REGRESSION_TESTS_USE_CSVSA" in os.environ else 65)
+            "jbmc") == 2
+        # Check there aren't any other traces
+        assert traces.count_traces() == 2

src/taint-slicer/instrumenter.cpp

@@ -600,6 +600,20 @@ void taint_instrumentert::instrument_instructions_with_shadow_variables(
         }
       }
       break;
+    case OTHER:
+      if(instr_it->code.get_statement() == ID_array_set)
+      {
+        codet &code = instr_it->code;
+        code.op0() = drive_access_path_through_super_classes(code.op0());
+        INVARIANT(
+          can_cast_type<pointer_typet>(code.op0().type()),
+          "first argument to array_set must be of pointer type");
+        code.op1() = make_or_update_initialiser(
+          code.op1(),
+          ns.follow(code.op0().type().subtype()),
+          get_instrumented_symbol_table(),
+          get_names_of_shadow_variables());
+      }
     default:
       break;
     }