[quandary] new issue type for untrusted variable length array creation

Summary: Previously, we didn't distinguish between stack-based buffer sinks and dynamic alllocation sinks.

Reviewed By: mbouaziz

Differential Revision: D6156334

fbshipit-source-id: 0482046

infer/src/base/IssueType.ml

@@ -309,4 +309,6 @@ let unsafe_guarded_by_access = from_string "UNSAFE_GUARDED_BY_ACCESS"
 
 let use_after_free = from_string "USE_AFTER_FREE"
 
+let untrusted_variable_length_array = from_string "UNTRUSTED_VARIABLE_LENGTH_ARRAY"
+
 let wrong_argument_number = from_string "Wrong_argument_number" ~hum:"Wrong Argument Number"

infer/src/base/IssueType.mli

@@ -218,4 +218,6 @@ val unsafe_guarded_by_access : t
 
 val use_after_free : t
 
+val untrusted_variable_length_array : t
+
 val wrong_argument_number : t

infer/src/quandary/ClangTrace.ml

@@ -148,22 +148,27 @@ module CppSource = Source.Make (SourceKind)
 
 module SinkKind = struct
   type t =
-    | Allocation  (** memory allocation *)
     | BufferAccess  (** read/write an array *)
+    | HeapAllocation  (** heap memory allocation *)
     | ShellExec  (** shell exec function *)
     | SQL  (** SQL query *)
+    | StackAllocation  (** stack memory allocation *)
     | Other  (** for testing or uncategorized sinks *)
     [@@deriving compare]
 
   let matches ~caller ~callee = Int.equal 0 (compare caller callee)
 
   let of_string = function
-    | "Allocation" ->
+    | "BufferAccess" ->
-        Allocation
+        BufferAccess
+    | "HeapAllocation" ->
+        HeapAllocation
     | "ShellExec" ->
         ShellExec
     | "SQL" ->
         SQL
+    | "StackAllocation" ->
+        StackAllocation
     | _ ->
         Other
 
@@ -226,7 +231,7 @@ module SinkKind = struct
         taint_all BufferAccess actuals
     | Typ.Procname.C _ when Typ.Procname.equal pname BuiltinDecl.__set_array_length ->
         (* called when creating a stack-allocated array *)
-        taint_nth 1 Allocation actuals
+        taint_nth 1 StackAllocation actuals
     | Typ.Procname.C _ -> (
       match Typ.Procname.to_string pname with
       | "execl" | "execlp" | "execle" | "execv" | "execve" | "execvp" | "system" ->
@@ -234,7 +239,7 @@ module SinkKind = struct
       | "popen" ->
           taint_nth 0 ShellExec actuals
       | ("brk" | "calloc" | "malloc" | "realloc" | "sbrk") when Config.developer_mode ->
-          taint_all Allocation actuals
+          taint_all HeapAllocation actuals
       | "strcpy" when Config.developer_mode ->
           (* warn if source array is tainted *)
           taint_nth 1 BufferAccess actuals
@@ -258,14 +263,16 @@ module SinkKind = struct
   let pp fmt kind =
     F.fprintf fmt
       ( match kind with
-      | Allocation ->
-          "Allocation"
       | BufferAccess ->
           "BufferAccess"
+      | HeapAllocation ->
+          "HeapAllocation"
       | ShellExec ->
           "ShellExec"
       | SQL ->
           "SQL"
+      | StackAllocation ->
+          "StackAllocation"
       | Other ->
           "Other" )
 
@@ -303,9 +310,13 @@ include Trace.Make (struct
     | (CommandLineFlag _ | EnvironmentVariable | File | Other), SQL ->
         (* untrusted flag, environment var, or file data flowing to SQL *)
         Some IssueType.sql_injection
-    | (CommandLineFlag _ | Endpoint _ | EnvironmentVariable | File), Allocation ->
+    | (CommandLineFlag _ | Endpoint _ | EnvironmentVariable | File | Other), HeapAllocation ->
-        (* untrusted data of any kind flowing to memory allocation *)
+        (* untrusted data of any kind flowing to heap allocation. this can cause crashes or DOS. *)
         Some IssueType.quandary_taint_error
+    | (CommandLineFlag _ | Endpoint _ | EnvironmentVariable | File | Other), StackAllocation ->
+        (* untrusted data of any kind flowing to stack buffer allocation. trying to allocate a stack
+           buffer that's too large will cause a stack overflow. *)
+        Some IssueType.untrusted_variable_length_array
     | Other, _ ->
         (* Other matches everything *)
         Some IssueType.quandary_taint_error

infer/tests/codetoanalyze/cpp/quandary/issues.exp

@@ -12,7 +12,7 @@ codetoanalyze/cpp/quandary/arrays.cpp, arrays::array_sink4_bad, 2, QUANDARY_TAIN
 codetoanalyze/cpp/quandary/arrays.cpp, arrays::memcpy_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to memcpy]
 codetoanalyze/cpp/quandary/arrays.cpp, arrays::memmove_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to memmove]
 codetoanalyze/cpp/quandary/arrays.cpp, arrays::memset_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to memset]
-codetoanalyze/cpp/quandary/arrays.cpp, arrays::stack_smash_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __set_array_length]
+codetoanalyze/cpp/quandary/arrays.cpp, arrays::stack_smash_bad, 2, UNTRUSTED_VARIABLE_LENGTH_ARRAY, [Return from __infer_taint_source,Call to __set_array_length]
 codetoanalyze/cpp/quandary/arrays.cpp, arrays::std_array_sink_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to std::array<int,2>_operator[]]
 codetoanalyze/cpp/quandary/arrays.cpp, arrays::std_string_sink_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to std::basic_string<char,std::char_traits<char>,std::allocator<char>>_operator[]]
 codetoanalyze/cpp/quandary/arrays.cpp, arrays::strcpy_bad, 2, QUANDARY_TAINT_ERROR, [Return from getenv,Call to strcpy]