Cwe 560 (#21)

* Initial version of CWE560 check

* CWE560 identifies calls to umask, missing the check of the umask calls.

* Initial version of CWE560 check

* CWE560 identifies calls to umask, missing the check of the umask calls.

* [cwe560] works for x64, fix function check_umask_call to detect on
other arches

* Initial version of CWE560 check

* CWE560 identifies calls to umask, missing the check of the umask calls.

* Initial version of CWE560 check

* [cwe560] works for x64, fix function check_umask_call to detect on
other arches

* Now working on the other architectures

* Refactored version of check for CWE 560 that work on several architectures. Added first unit tests for the checkers code base

* Fixes some dune warnings.

* Added CWE 560 to CHANGES.md. Fixes another dune warning.

* Requested change: Private module as a wrapper for unit tests

CHANGES.md

@@ -12,6 +12,7 @@
 -   Switched C build system from make to scons (PR #16)
 -   Added type inference pass (PR #14, #18)
 -   Added unit tests to test suite (PR #14)
+-   Added check for CWE-560 (Use of umask() with chmod-style Argument) (PR #21)
 
 0.1 (2018-10-08)
 =====

README.md

@@ -25,6 +25,7 @@ Its main focus are ELF binaries that are commonly found on Linux and Unix operat
 -   [CWE-457](https://cwe.mitre.org/data/definitions/457.html): Use of Uninitialized Variable
 -   [CWE-467](https://cwe.mitre.org/data/definitions/467.html): Use of sizeof() on a Pointer Type
 -   [CWE-476](https://cwe.mitre.org/data/definitions/476.html): NULL Pointer Dereference
+-   [CWE-560](https://cwe.mitre.org/data/definitions/560.html): Use of umask() with chmod-style Argument
 -   [CWE-676](https://cwe.mitre.org/data/definitions/676.html): Use of Potentially Dangerous Function
 -   [CWE-782](https://cwe.mitre.org/data/definitions/782.html): Exposed IOCTL with Insufficient Access Control
 -   [CWE-787](https://cwe.mitre.org/data/definitions/787.html): Out-of-bounds Write

plugins/cwe_checker/cwe_checker.ml

@@ -25,6 +25,7 @@ let known_modules = [{cwe_func = Cwe_190.check_cwe; name = Cwe_190.name; version
                      {cwe_func = Cwe_457.check_cwe; name = Cwe_457.name; version = Cwe_457.version; requires_pairs = false; has_parameters = false};
                      {cwe_func = Cwe_467.check_cwe; name = Cwe_467.name; version = Cwe_467.version; requires_pairs = false; has_parameters = false};
                      {cwe_func = Cwe_476.check_cwe; name = Cwe_476.name; version = Cwe_476.version; requires_pairs = false; has_parameters = true};
+                      {cwe_func = Cwe_560.check_cwe; name = Cwe_560.name; version = Cwe_560.version; requires_pairs = false; has_parameters = false};
                      {cwe_func = Cwe_676.check_cwe; name = Cwe_676.name; version = Cwe_676.version; requires_pairs = false; has_parameters = false};
                      {cwe_func = Cwe_782.check_cwe; name = Cwe_782.name; version = Cwe_782.version; requires_pairs = false; has_parameters = false}]
 

src/checkers/cwe_560.ml

@@ -0,0 +1,61 @@
+open Core_kernel
+open Bap.Std
+
+let name = "CWE560"
+let version = "0.1"
+
+let upper_bound_of_correct_umask_arg_value = 100
+let upper_bound_of_correct_chmod_arg_value = 1000
+
+ let collect_int_values = Exp.fold ~init:[] (object
+          inherit [word list] Exp.visitor
+          method! enter_int x addrs = x :: addrs
+        end)
+
+let is_chmod_style_arg umask_arg =
+  umask_arg  > upper_bound_of_correct_umask_arg_value && umask_arg < upper_bound_of_correct_chmod_arg_value
+
+let check_umask_arg tid_map blk w =
+  try
+    let umask_arg = Word.to_int_exn w in
+    if is_chmod_style_arg umask_arg then
+      Log_utils.warn "[%s] {%s} (Use of umask() with chmod-style Argument) Function %s calls umask with argument %d"
+        name
+        version
+        (Address_translation.translate_tid_to_assembler_address_string (Term.tid blk) tid_map)
+        umask_arg
+  with _ -> Log_utils.error "Caught exception in module [CWE560]."
+
+let check_umask_callsite tid_map blk =
+  Seq.iter (Term.enum def_t blk) ~f:(fun d ->
+   let rhs = Def.rhs d in
+   let int_values = collect_int_values rhs in
+   List.iter int_values ~f:(fun x -> check_umask_arg tid_map blk x)
+  )
+
+let blk_calls_umask sym_umask blk =
+  Term.enum jmp_t blk
+  |> Seq.exists ~f:(fun callsite -> Symbol_utils.calls_callsite_symbol callsite sym_umask)
+
+let check_subfunction program tid_map sym_umask sub =
+  if Symbol_utils.sub_calls_symbol program sub "umask" then
+    Term.enum blk_t sub
+    |> Seq.filter ~f:(fun blk -> blk_calls_umask sym_umask blk)
+    |> Seq.iter ~f:(fun blk -> check_umask_callsite tid_map blk)
+  else
+    ()
+
+let check_subfunctions program tid_map sym_umask =
+  Seq.iter (Term.enum sub_t program) ~f:(fun sub -> check_subfunction program tid_map sym_umask sub)
+
+let check_cwe program _ tid_map _ _ =
+    let sym = Symbol_utils.get_symbol_of_string program "umask" in
+    match sym with
+    | None -> ()
+    | Some sym_umask -> check_subfunctions program tid_map sym_umask
+
+
+(* Functions made available for unit tests *)
+module Private = struct
+  let is_chmod_style_arg = is_chmod_style_arg
+end

src/checkers/cwe_560.mli

@@ -0,0 +1,23 @@
+(** CWE-560 (Use of umask() with chmod-style Argument)
+https://cwe.mitre.org/data/definitions/560.html
+
+The program uses the system call umask(2) with arguements for chmod(2). For instance,
+instead of a reasonable value like 0022 a value like 0666 is passed. This may result wrong
+read and/or write access to files and directories, which could be utilized to bypass
+protection mechanisms.
+
+This check looks for umask calls and checks if they have a reasonable value, i.e. smaller than
+a certain value, currently set to 1000 and greater than a reasonable value for umask, currently set to 100.
+A future version should include a proper data flow analysis to track the first argument since the current
+version considers all immediate values of an umask callsite's basic block.
+*)
+val name : string
+val version : string
+
+val check_cwe : Bap.Std.program Bap.Std.term -> Bap.Std.project -> Bap.Std.word Bap.Std.Tid.Map.t -> string list list -> string list -> unit
+
+
+(* functions made available for unit tests: *)
+module Private : sig
+  val is_chmod_style_arg : int -> bool
+end

src/utils/symbol_utils.ml

@@ -19,6 +19,15 @@ let build_symbols symbol_names prog =
       | Some _ -> true
       | _ -> false)
 
+let get_symbol_of_string prog name =
+  let symbol_address = find_symbol prog name in
+  match symbol_address with
+  | Some _ -> Some ({
+                          address = symbol_address
+                          ; name = name
+                          })
+  | None -> None
+
 let get_symbol tid symbols =
   List.find symbols ~f:(
     fun symbol -> match symbol.address with
@@ -65,6 +74,20 @@ let sub_calls_symbol prog sub symbol_name =
   end
   | _ -> false
 
+let calls_callsite_symbol jmp symbol =
+  match Jmp.kind jmp with
+  | Goto _ | Ret _ | Int (_,_) -> false
+  | Call dst -> begin
+      match Call.target dst with
+      | Direct tid -> begin
+            match symbol.address with
+            | Some symbol_tid -> tid = symbol_tid
+            | None -> false
+          end
+      | _ -> false
+    end
+
+
 type concrete_call =
   {
     call_site : tid;

src/utils/symbol_utils.mli

@@ -1,13 +1,16 @@
 (** This module implements functionality to work with symbols (e.g. malloc).*)
 
 type concrete_call = {
-  call_site : Bap.Std.tid;
-  symbol_address : Bap.Std.tid;
-  name : string;
+  call_site : Bap.Std.tid
+  ; symbol_address : Bap.Std.tid
+  ; name : string;
 }
 
 (** This type represents a symbol like malloc or memcpy. *)
-type symbol = { address : Bap.Std.tid option; name : string; }
+type symbol = {
+  address : Bap.Std.tid option
+  ; name : string;
+}
 
 (** Finds a symbol string in a program and returns its IR address (tid). *)
 val find_symbol : Bap.Std.program Bap.Std.term -> string -> Bap.Std.tid option
@@ -19,13 +22,19 @@ val build_symbols : string list -> Bap.Std.program Bap.Std.term -> symbol list
 (** Gets a symbol from a symbol list *)
 val get_symbol : Bap.Std.tid -> symbol list -> symbol option
 
+(** Gets a symbol from a string *)
+val get_symbol_of_string : Bap.Std.program Bap.Std.term -> string -> symbol option
+
 (** Given a JMP and symbol list, it returns its name as string.
    Use only if you are sure that the JMP calls a symbol. *)
 val get_symbol_name_from_jmp : Bap.Std.Jmp.t -> symbol list -> string
 
 (** Checks if a subfunction calls a symbol. *)
 val sub_calls_symbol: Bap.Std.program Bap.Std.term -> Bap.Std.sub Bap.Std.term -> string -> bool
 
+(** Checks if a callsite calls a symbol *)
+val calls_callsite_symbol : Bap.Std.Jmp.t -> symbol -> bool
+
 (** This function finds all (direct) calls in a program. It returns a list of tuples of (callsite, address).*)
 val call_finder : (Bap.Std.tid * Bap.Std.tid) list Bap.Std.Term.visitor
 

test/acceptance/test_cwe560.py

@@ -0,0 +1,36 @@
+import unittest
+import cwe_checker_testlib
+
+
+class TestCwe560(unittest.TestCase):
+
+    def setUp(self):
+        self.target = '560'
+        self.string = b'Use of umask() with chmod-style Argument'
+
+    @unittest.skip("Args of umask to not seem to be found by BAP. Investigate in the future")
+    def test_cwe560_01_arm(self):
+        expect_res = 1
+        res = cwe_checker_testlib.execute_and_check_occurence(self.target, self.target, 'arm', self.string)
+        self.assertEqual(res, expect_res)
+
+    def test_cwe560_01_x86(self):
+        expect_res = 1
+        res = cwe_checker_testlib.execute_and_check_occurence(self.target, self.target, 'x86', self.string)
+        self.assertEqual(res, expect_res)
+
+    def test_cwe560_01_x64(self):
+        expect_res = 1
+        res = cwe_checker_testlib.execute_and_check_occurence(self.target, self.target, 'x64', self.string)
+        self.assertEqual(res, expect_res)
+
+    @unittest.skip("Depends on proper MIPS support in BAP")
+    def test_cwe560_01_mips(self):
+        expect_res = 1
+        res = cwe_checker_testlib.execute_and_check_occurence(self.target, self.target, 'mips', self.string)
+        self.assertEqual(res, expect_res)
+
+    def test_cwe560_01_ppc(self):
+        expect_res = 1
+        res = cwe_checker_testlib.execute_and_check_occurence(self.target, self.target, 'ppc', self.string)
+        self.assertEqual(res, expect_res)

test/artificial_samples/cwe_560.c

@@ -0,0 +1,24 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+void umask_incorrect(){
+        umask(666);
+        int fd = open("some_random_file", O_CREAT|O_WRONLY, 0666);
+        close(fd);
+}
+
+void umask_correct(){
+        umask(022);
+        int fd = open("some_random_file", O_CREAT|O_WRONLY, 0666);
+        close(fd);
+}
+
+int main(){
+        umask_correct();
+        umask_incorrect();
+        return 0;
+}

test/unit/Makefile

@@ -1,6 +1,6 @@
 all:
 	bapbundle remove cwe_checker_unit_tests.plugin
-	bapbuild -r -Is analysis,utils cwe_checker_unit_tests.plugin -pkgs core,alcotest,yojson,unix,ppx_jane,cwe_checker_core
+	bapbuild -r -Is analysis,checkers,utils cwe_checker_unit_tests.plugin -pkgs core,alcotest,yojson,unix,ppx_jane,cwe_checker_core
 	bapbundle install cwe_checker_unit_tests.plugin
 	bap ../artificial_samples/build/arrays_x64.out --pass=cwe-checker-unit-tests
 	bapbundle remove cwe_checker_unit_tests.plugin

test/unit/checkers/cwe_560_test.ml

@@ -0,0 +1,18 @@
+open Bap.Std
+open Core_kernel
+open Cwe_checker_core
+
+let check msg x = Alcotest.(check bool) msg true x
+
+let test_is_chmod_style_arg_with_umask_arg () : unit =
+  let res = Cwe_560.Private.is_chmod_style_arg 022 in
+  check "empty" (res = false)
+
+let test_is_chmod_style_arg_with_chmod_arg () : unit =
+  let res = Cwe_560.Private.is_chmod_style_arg 666 in
+  check "empty" (res = true)
+
+let tests = [
+  "Is chmod style argument with umask argument?", `Quick, test_is_chmod_style_arg_with_umask_arg;
+  "Is chmod style argument with chmod argument?", `Quick, test_is_chmod_style_arg_with_chmod_arg;
+]

test/unit/checkers/cwe_560_test.mli

@@ -0,0 +1,4 @@
+open Bap.Std
+open Core_kernel
+
+val tests: unit Alcotest.test_case list

test/unit/cwe_checker_unit_tests.ml

@@ -9,6 +9,7 @@ let run_tests project =
     "Mem_region_tests", Mem_region_test.tests;
     "Type_inference_tests", Type_inference_test.tests;
     "Cconv_tests", Cconv_test.tests;
+    "CWE_560_tests", Cwe_560_test.tests;
      ]
 
 let () =