Dedup incident report (#52)

Improved the CWE reports generated by emulation based checks.

plugins/cwe_checker_emulation/cwe_checker_emulation.ml

@@ -121,7 +121,7 @@ end
 
 (** Main logic of program:
 - we monitor all 'incident' related events
-- for all subroutins we fork a Primus machine
+- for all subroutines we fork a Primus machine
 - all monitored events are collected globally
 - after the last Primus machine has terminated we report all observed incidents *)
 let main json_output file_output proj =
@@ -137,6 +137,9 @@ let main json_output file_output proj =
      info "program terminated by a signal: %s" (Primus.Exn.to_string exn);
   end;
   analyze_events ();
+  Incident_reporter.parse_reports ();
+  Incident_reporter.report_cwe ();
+  Incident_reporter.report_unknown_incidents ();
   if json_output then
     begin
       match Project.get proj filename with
@@ -146,6 +149,7 @@ let main json_output file_output proj =
   else
     Log_utils.emit_native file_output
 
+
 module Cmdline = struct
   open Config
   let json_output = flag "json" ~doc:"Outputs the result as JSON."

plugins/cwe_checker_emulation/incident_reporter.ml

@@ -7,6 +7,13 @@ let version = "0.1"
 (** Keeps track of reported events so that events are not reported multiple times. *)
 let reported_events = ref (String.Set.empty)
 
+(** We may want to get the number of emulated CWEs from a central point for scalability *)
+let collected_locations = Hashtbl.create (module Int) ~size:4
+let known_incidents = Hashtbl.of_alist_exn (module Int) [(125, "(Out-of-bounds Read)"); (787, "(Out-of-bounds Write)"); (415, "(Double Free)"); (416, "(Use After Free)")]
+
+let cwe_incidents = ref [||]
+let unknown_cwe_incidents = ref [||]
+
 (** Builds a string of a path of addresses. *)
 let build_location_path locations =
   let rec internal locations path_str =
@@ -25,30 +32,59 @@ let map_id_to_location id loc_tbl =
 (** Translates a list of incident ids to a list of concrete addresses. *)
 let get_incident_locations_from_ids ids location_tbl =
   let incident_locations = ref [] in
-      Sexplib__Sexp_with_layout.List.iter ids ~f:(fun id ->  incident_locations := (map_id_to_location (Sexp.to_string id) location_tbl)::(!incident_locations)); !incident_locations
-
-let report_cwe_125 location_path =
-  let description = sprintf "(Out-of-bounds Read) %s" location_path in
-  let cwe_warning = cwe_warning_factory "CWE125" version ~other:[["path";location_path]] description in
-  collect_cwe_warning cwe_warning;
-  let description = sprintf "(Out-of-bounds Write) %s" location_path in
-  let cwe_warning = cwe_warning_factory "CWE787" version ~other:[["path";location_path]] description in
-  collect_cwe_warning cwe_warning
-
-let report_cwe_415 location_path =
-  let description = sprintf "(Double Free) %s" location_path in
-  let cwe_warning = cwe_warning_factory "CWE415" version ~other:[["path";location_path]] description in
-  collect_cwe_warning cwe_warning
-
-let report_cwe_416 location_path =
-  let description = sprintf "(Use After Free) %s" location_path in
-  let cwe_warning = cwe_warning_factory "CWE416" version ~other:[["path";location_path]] description in
-  collect_cwe_warning cwe_warning
-
-let report_cwe_unknown location_path incident_str =
-  let description = sprintf "(%s) %s" incident_str location_path in
-  let cwe_warning = cwe_warning_factory incident_str version ~other:[["path";location_path]] description in
-  collect_cwe_warning cwe_warning
+  Sexplib__Sexp_with_layout.List.iter ids ~f:(fun id ->  incident_locations := (map_id_to_location (Sexp.to_string id) location_tbl)::(!incident_locations)); !incident_locations
+
+
+let build_description (incident_str : string) (end_point : string) (paths : string list) : string =
+  let pretty_paths = ref "" in
+  pretty_paths := !pretty_paths^end_point^"\n";
+  List.iter ~f:(fun path ->
+    let clean_path = String.drop_suffix path 3 in
+    pretty_paths := !pretty_paths^"\n  "^clean_path
+  ) paths;
+  sprintf "%s %s \n" incident_str !pretty_paths
+
+
+let report_cwe _ =
+  Array.iter ~f:(fun (id, loc_hash) ->
+    let incident_str = (Hashtbl.find_exn known_incidents id) in
+    Hashtbl.iter_keys loc_hash ~f:(fun end_point ->
+      let paths = (Hashtbl.find_multi loc_hash end_point) in
+      let description = build_description incident_str end_point paths in
+      let other = List.map ~f:(fun path ->
+        let clean_path = String.drop_suffix path 3 in
+        ["path"; clean_path]
+      ) paths in
+      let cwe = sprintf "CWE%d" id in
+      collect_cwe_warning (cwe_warning_factory cwe version ~addresses:[end_point] ~other:other description)
+    )
+  ) !cwe_incidents
+
+
+let report_unknown_incidents _ =
+  Array.iter ~f:(fun (path, inc) ->
+    let description = inc ^ " " ^ path in
+    collect_cwe_warning (cwe_warning_factory inc version ~other:[["path"; path]] description)
+  ) !unknown_cwe_incidents
+
+
+let collect_known_incidents (cwe : int) (execution_path : string) =
+  Hashtbl.add_multi collected_locations ~key:cwe ~data:(String.rsplit2_exn execution_path ~on:' ')
+
+
+let collect_unknown_incidents (path_inc : string * string) =
+  unknown_cwe_incidents := Array.append !unknown_cwe_incidents [|path_inc|]
+
+
+let parse_reports _ =
+  Hashtbl.iter_keys collected_locations ~f:(fun id ->
+    let loc_hashtbl = Hashtbl.create (module String) ~size:3 in
+    List.iter ~f:(fun (path, end_point) ->
+      Hashtbl.add_multi loc_hashtbl ~key:end_point ~data:path
+    ) (Hashtbl.find_multi collected_locations id);
+    cwe_incidents := Array.append !cwe_incidents [|(id, loc_hashtbl)|]
+  )
+
 
 (** Reports an incident. *)
 let report incident location_tbl =
@@ -66,10 +102,10 @@ let report incident location_tbl =
         begin
           reported_events := Set.add !reported_events location_path;
             match incident_str with
-            | "memcheck-out-of-bound" -> report_cwe_125 location_path
-            | "memcheck-double-release" -> report_cwe_415 location_path
-            | "memcheck-use-after-release" -> report_cwe_416 location_path
-            | _ -> report_cwe_unknown location_path incident_str
+              | "memcheck-out-of-bound" -> collect_known_incidents 125 location_path
+              | "memcheck-double-release" -> collect_known_incidents 415 location_path
+              | "memcheck-use-after-release" -> collect_known_incidents 416 location_path
+              | _ -> collect_unknown_incidents (location_path, incident_str)
           end
     end
   | __ -> failwith "Strange incident sexp encountered"

test/acceptance/test_cwe415.py

@@ -30,7 +30,7 @@ class TestCwe415(unittest.TestCase):
         self.assertEqual(res, expect_res)
 
     def test_cwe415_01_x86_clang(self):
-        expect_res = 7
+        expect_res = 3
         res = cwe_checker_testlib.execute_emulation_and_check_occurence(
             self.target, self.target, 'x86', 'clang', self.string)
         self.assertEqual(res, expect_res)

test/acceptance/test_cwe416.py

@@ -30,7 +30,7 @@ class TestCwe416(unittest.TestCase):
         self.assertEqual(res, expect_res)
 
     def test_cwe416_01_x86_clang(self):
-        expect_res = 3
+        expect_res = 1
         res = cwe_checker_testlib.execute_emulation_and_check_occurence(
             self.target, self.target, 'x86', 'clang', self.string)
         self.assertEqual(res, expect_res)