[quandary] file creation as sink

Reviewed By: jeremydubreil, mbouaziz

Differential Revision: D6486526

fbshipit-source-id: cad09f1

infer/src/base/IssueType.ml

@@ -321,6 +321,10 @@ let unsafe_guarded_by_access = from_string "UNSAFE_GUARDED_BY_ACCESS"
 
 let use_after_free = from_string "USE_AFTER_FREE"
 
+let untrusted_file = from_string "UNTRUSTED_FILE"
+
+let untrusted_file_risk = from_string "UNTRUSTED_FILE_RISK"
+
 let untrusted_variable_length_array = from_string "UNTRUSTED_VARIABLE_LENGTH_ARRAY"
 
 let user_controlled_sql_risk = from_string "USER_CONTROLLED_SQL_RISK"

infer/src/base/IssueType.mli

@@ -230,6 +230,10 @@ val unsafe_guarded_by_access : t
 
 val use_after_free : t
 
+val untrusted_file : t
+
+val untrusted_file_risk : t
+
 val untrusted_variable_length_array : t
 
 val user_controlled_sql_risk : t

infer/src/quandary/ClangTrace.ml

@@ -16,7 +16,7 @@ module SourceKind = struct
     | CommandLineFlag of Var.t  (** source that was read from a command line flag *)
     | Endpoint of (Mangled.t * Typ.desc)  (** source originating from formal of an endpoint *)
     | EnvironmentVariable  (** source that was read from an environment variable *)
-    | File  (** source that was read from a file *)
+    | ReadFile  (** source that was read from a file *)
     | Other  (** for testing or uncategorized sources *)
     | UserControlledEndpoint of (Mangled.t * Typ.desc)
         (** source originating from formal of an endpoint that is known to hold user-controlled data *)
@@ -31,8 +31,8 @@ module SourceKind = struct
         Endpoint (Mangled.from_string "NONE", Typ.Tvoid)
     | "EnvironmentVariable" ->
         EnvironmentVariable
-    | "File" ->
+    | "ReadFile" ->
-        File
+        ReadFile
     | "UserControlledEndpoint" ->
         Endpoint (Mangled.from_string "NONE", Typ.Tvoid)
     | _ ->
@@ -73,7 +73,7 @@ module SourceKind = struct
         with
         | ( ["std"; ("basic_istream" | "basic_iostream")]
           , ("getline" | "read" | "readsome" | "operator>>") ) ->
-            Some (File, Some 1)
+            Some (ReadFile, Some 1)
         | _ ->
             get_external_source qualified_pname )
     | Typ.Procname.C _ when Typ.Procname.equal pname BuiltinDecl.__global_access
@@ -159,7 +159,7 @@ module SourceKind = struct
         F.fprintf fmt "Endpoint[%s]" (Mangled.to_string formal_name)
     | EnvironmentVariable ->
         F.fprintf fmt "EnvironmentVariable"
-    | File ->
+    | ReadFile ->
         F.fprintf fmt "File"
     | CommandLineFlag var ->
         F.fprintf fmt "CommandLineFlag[%a]" Var.pp var
@@ -175,6 +175,7 @@ module CppSource = Source.Make (SourceKind)
 module SinkKind = struct
   type t =
     | BufferAccess  (** read/write an array *)
+    | CreateFile  (** create/open a file *)
     | HeapAllocation  (** heap memory allocation *)
     | ShellExec  (** shell exec function *)
     | SQL  (** SQL query *)
@@ -187,6 +188,8 @@ module SinkKind = struct
   let of_string = function
     | "BufferAccess" ->
         BufferAccess
+    | "CreateFile" ->
+        CreateFile
     | "HeapAllocation" ->
         HeapAllocation
     | "ShellExec" ->
@@ -246,9 +249,16 @@ module SinkKind = struct
       || String.is_substring ~substring:"string" typename
     in
     match pname with
-    | Typ.Procname.ObjC_Cpp _ -> (
+    | Typ.Procname.ObjC_Cpp cpp_name -> (
-      match Typ.Procname.get_method pname with
+      match
-      | "operator[]" when Config.developer_mode && is_buffer_like pname ->
+        ( QualifiedCppName.to_list
+            (Typ.Name.unqualified_name (Typ.Procname.objc_cpp_get_class_type_name cpp_name))
+        , Typ.Procname.get_method pname )
+      with
+      | ( ["std"; ("basic_fstream" | "basic_ifstream" | "basic_ofstream")]
+        , ("basic_fstream" | "basic_ifstream" | "basic_ofstream" | "open") ) ->
+          taint_nth 1 CreateFile actuals
+      | _, "operator[]" when Config.developer_mode && is_buffer_like pname ->
           taint_nth 1 BufferAccess actuals
       | _ ->
           get_external_sink pname actuals )
@@ -260,12 +270,18 @@ module SinkKind = struct
         taint_nth 1 StackAllocation actuals
     | Typ.Procname.C _ -> (
       match Typ.Procname.to_string pname with
+      | "creat" | "fopen" | "freopen" | "open" ->
+          taint_nth 0 CreateFile actuals
       | "execl" | "execlp" | "execle" | "execv" | "execve" | "execvp" | "system" ->
           taint_all ShellExec actuals
+      | "openat" ->
+          taint_nth 1 CreateFile actuals
       | "popen" ->
           taint_nth 0 ShellExec actuals
       | ("brk" | "calloc" | "malloc" | "realloc" | "sbrk") when Config.developer_mode ->
           taint_all HeapAllocation actuals
+      | "rename" ->
+          taint_all CreateFile actuals
       | "strcpy" when Config.developer_mode ->
           (* warn if source array is tainted *)
           taint_nth 1 BufferAccess actuals
@@ -291,6 +307,8 @@ module SinkKind = struct
       ( match kind with
       | BufferAccess ->
           "BufferAccess"
+      | CreateFile ->
+          "CreateFile"
       | HeapAllocation ->
           "HeapAllocation"
       | ShellExec ->
@@ -358,6 +376,10 @@ include Trace.Make (struct
     | _ when List.mem sanitizers Sanitizer.All ~equal:Sanitizer.equal ->
         (* the All sanitizer clears any form of taint; don't report *)
         None
+    | UserControlledEndpoint (_, typ), CreateFile ->
+        Option.some_if (is_injection_possible ~typ sanitizers) IssueType.untrusted_file
+    | Endpoint (_, typ), CreateFile ->
+        Option.some_if (is_injection_possible ~typ sanitizers) IssueType.untrusted_file_risk
     | UserControlledEndpoint (_, typ), SQL ->
         if is_injection_possible ~typ sanitizers then Some IssueType.sql_injection
         else
@@ -382,24 +404,26 @@ include Trace.Make (struct
     | Endpoint _, (BufferAccess | HeapAllocation | StackAllocation) ->
         (* may want to report this in the future, but don't care for now *)
         None
-    | (CommandLineFlag _ | EnvironmentVariable | File | Other), BufferAccess ->
+    | (CommandLineFlag _ | EnvironmentVariable | ReadFile | Other), BufferAccess ->
         (* untrusted flag, environment var, or file data flowing to buffer *)
         Some IssueType.quandary_taint_error
-    | (CommandLineFlag _ | EnvironmentVariable | File | Other), ShellExec ->
+    | (CommandLineFlag _ | EnvironmentVariable | ReadFile | Other), ShellExec ->
         (* untrusted flag, environment var, or file data flowing to shell *)
         Option.some_if (is_injection_possible sanitizers) IssueType.shell_injection
-    | (CommandLineFlag _ | EnvironmentVariable | File | Other), SQL ->
+    | (CommandLineFlag _ | EnvironmentVariable | ReadFile | Other), SQL ->
         (* untrusted flag, environment var, or file data flowing to SQL *)
         Option.some_if (is_injection_possible sanitizers) IssueType.sql_injection
-    | ( (CommandLineFlag _ | UserControlledEndpoint _ | EnvironmentVariable | File | Other)
+    | ( (CommandLineFlag _ | UserControlledEndpoint _ | EnvironmentVariable | ReadFile | Other)
       , HeapAllocation ) ->
         (* untrusted data of any kind flowing to heap allocation. this can cause crashes or DOS. *)
         Some IssueType.quandary_taint_error
-    | ( (CommandLineFlag _ | UserControlledEndpoint _ | EnvironmentVariable | File | Other)
+    | ( (CommandLineFlag _ | UserControlledEndpoint _ | EnvironmentVariable | ReadFile | 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
+    | (CommandLineFlag _ | EnvironmentVariable | ReadFile), CreateFile ->
+        None
     | Other, _ ->
         (* Other matches everything *)
         Some IssueType.quandary_taint_error

infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp

@@ -7,6 +7,8 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  */
 
+#include <fcntl.h>
+#include <fstream>
 #include <string>
 
 extern void __infer_sql_sink(std::string);
@@ -73,6 +75,33 @@ class Service1 : facebook::fb303::cpp2::FacebookServiceSvIf {
     system(formal.s.c_str());
   }
 
+  void open_or_create_c_style_file_bad(const char* filename) {
+    open(filename, 0);
+    openat(1, filename, 2);
+    creat(filename, 3);
+    fopen(filename, "w");
+    freopen(filename, "w", nullptr);
+    rename(filename, "mud");
+  }
+
+  void ofstream_open_file_bad(std::string filename) {
+    std::ofstream file1(filename);
+    std::ofstream file2;
+    file2.open(filename);
+  }
+
+  void ifstream_open_file_bad(std::string filename) {
+    std::ifstream file1(filename);
+    std::ifstream file2;
+    file2.open(filename);
+  }
+
+  void fstream_open_file_bad(std::string filename) {
+    std::fstream file1(filename);
+    std::fstream file2;
+    file2.open(filename);
+  }
+
   void FP_service1_endpoint_struct_int_field_ok(request formal) {
     system(std::to_string(formal.i).c_str());
   }

infer/tests/codetoanalyze/cpp/quandary/files.cpp

@@ -61,4 +61,5 @@ void read_file_call_exec_bad5(std::iostream is, int length) {
     execle(buffer, NULL);
   }
 }
+
 }

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

@@ -37,6 +37,18 @@ codetoanalyze/cpp/quandary/basics.cpp, basics::via_field_bad2, 2, QUANDARY_TAINT
 codetoanalyze/cpp/quandary/basics.cpp, basics::via_passthrough_bad1, 4, QUANDARY_TAINT_ERROR, [Return from basics::Obj_string_source,Call to basics::Obj_string_sink]
 codetoanalyze/cpp/quandary/basics.cpp, basics::via_passthrough_bad2, 3, QUANDARY_TAINT_ERROR, [Return from basics::Obj_string_source,Call to basics::Obj_string_sink]
 codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_FP_service1_endpoint_struct_int_field_ok, 1, REMOTE_CODE_EXECUTION_RISK, [Return from endpoints::Service1_FP_service1_endpoint_struct_int_field_ok,Call to system]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_fstream_open_file_bad, 1, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_fstream_open_file_bad,Call to std::basic_fstream<char,std::char_traits<char>>_basic_fstream]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_fstream_open_file_bad, 3, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_fstream_open_file_bad,Call to std::basic_fstream<char,std::char_traits<char>>_open]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_ifstream_open_file_bad, 1, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_ifstream_open_file_bad,Call to std::basic_ifstream<char,std::char_traits<char>>_basic_ifstream]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_ifstream_open_file_bad, 3, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_ifstream_open_file_bad,Call to std::basic_ifstream<char,std::char_traits<char>>_open]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_ofstream_open_file_bad, 1, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_ofstream_open_file_bad,Call to std::basic_ofstream<char,std::char_traits<char>>_basic_ofstream]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_ofstream_open_file_bad, 3, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_ofstream_open_file_bad,Call to std::basic_ofstream<char,std::char_traits<char>>_open]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_open_or_create_c_style_file_bad, 1, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_open_or_create_c_style_file_bad,Call to open]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_open_or_create_c_style_file_bad, 2, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_open_or_create_c_style_file_bad,Call to openat]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_open_or_create_c_style_file_bad, 3, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_open_or_create_c_style_file_bad,Call to creat]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_open_or_create_c_style_file_bad, 4, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_open_or_create_c_style_file_bad,Call to fopen]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_open_or_create_c_style_file_bad, 5, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_open_or_create_c_style_file_bad,Call to freopen]
+codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_open_or_create_c_style_file_bad, 6, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_open_or_create_c_style_file_bad,Call to rename]
 codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_sanitized_sql_bad, 2, USER_CONTROLLED_SQL_RISK, [Return from endpoints::Service1_sanitized_sql_bad,Call to __infer_sql_sink]
 codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_service1_endpoint_bad, 2, REMOTE_CODE_EXECUTION_RISK, [Return from endpoints::Service1_service1_endpoint_bad,Call to system]
 codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_service1_endpoint_struct_string_field_bad, 1, REMOTE_CODE_EXECUTION_RISK, [Return from endpoints::Service1_service1_endpoint_struct_string_field_bad,Call to system]