[quandary] thrift services as sources in Java

Reviewed By: jeremydubreil

Differential Revision: D6813966

fbshipit-source-id: 8cddca6

infer/src/checkers/annotations.ml

@@ -93,6 +93,8 @@ let thread_confined = "ThreadConfined"
 
 let thread_safe = "ThreadSafe"
 
+let thrift_service = "ThriftService"
+
 let true_on_null = "TrueOnNull"
 
 let ui_thread = "UiThread"
@@ -178,6 +180,8 @@ let ia_is_synchronized_collection ia = ia_ends_with ia synchronized_collection
 
 let ia_is_thread_safe ia = ia_ends_with ia thread_safe
 
+let ia_is_thrift_service ia = ia_ends_with ia thrift_service
+
 let ia_is_true_on_null ia = ia_ends_with ia true_on_null
 
 let ia_is_initializer ia = ia_ends_with ia initializer_

infer/src/checkers/annotations.mli

@@ -110,6 +110,8 @@ val ia_is_thread_safe : Annot.Item.t -> bool
 
 val ia_is_thread_confined : Annot.Item.t -> bool
 
+val ia_is_thrift_service : Annot.Item.t -> bool
+
 val ia_is_ui_thread : Annot.Item.t -> bool
 
 val ia_is_volatile : Annot.Item.t -> bool

infer/src/quandary/JavaTrace.ml

@@ -14,6 +14,7 @@ module L = Logging
 module SourceKind = struct
   type t =
     | DrawableResource of Pvar.t  (** Drawable resource ID read from a global *)
+    | Endpoint of (Mangled.t * Typ.desc)  (** source originating from formal of an endpoint *)
     | Intent  (** external Intent or a value read from one *)
     | IntentFromURI  (** Intent created from a URI *)
     | Other  (** for testing or uncategorized sources *)
@@ -25,6 +26,8 @@ module SourceKind = struct
   let matches ~caller ~callee = Int.equal 0 (compare caller callee)
 
   let of_string = function
+    | "Endpoint" ->
+        Endpoint (Mangled.from_string "NONE", Typ.Tvoid)
     | "Intent" ->
         Intent
     | "IntentFromURI" ->
@@ -150,6 +153,20 @@ module SourceKind = struct
       in
       List.map ~f:taint_formal_with_types formals
     in
+    (* taint all formals except for [this] *)
+    let taint_all_but_this ~make_source =
+      List.map
+        ~f:(fun (name, typ) ->
+          let taint =
+            match Mangled.to_string name with
+            | "this" ->
+                None
+            | _ ->
+                Some (make_source name typ.Typ.desc)
+          in
+          (name, typ, taint) )
+        (Procdesc.get_formals pdesc)
+    in
     let formals = Procdesc.get_formals pdesc in
     match Procdesc.get_proc_name pdesc with
     | Typ.Procname.Java java_pname -> (
@@ -199,7 +216,15 @@ module SourceKind = struct
               , ("onJsAlert" | "onJsBeforeUnload" | "onJsConfirm" | "onJsPrompt") ) ->
                 Some (taint_formals_with_types ["java.lang.String"] UserControlledURI formals)
             | _ ->
-                None
+              match Tenv.lookup tenv typename with
+              | Some typ ->
+                  if Annotations.struct_typ_has_annot typ Annotations.ia_is_thrift_service then
+                    (* assume every non-this formal of a Thrift service is tainted *)
+                    (* TODO: may not want to taint numbers or Enum's *)
+                    Some (taint_all_but_this ~make_source:(fun name desc -> Endpoint (name, desc)))
+                  else None
+              | _ ->
+                  None
           in
           match
             PatternMatch.supertype_find_map_opt tenv taint_matching_supertype
@@ -219,6 +244,8 @@ module SourceKind = struct
       ( match kind with
       | DrawableResource pvar ->
           Pvar.to_string pvar
+      | Endpoint (formal_name, _) ->
+          F.asprintf "Endpoint[%s]" (Mangled.to_string formal_name)
       | Intent ->
           "Intent"
       | IntentFromURI ->
@@ -457,19 +484,20 @@ include Trace.Make (struct
     | _ when not (List.is_empty sanitizers) ->
         (* assume any sanitizer clears all forms of taint *)
         None
-    | (Intent | UserControlledString | UserControlledURI), CreateIntent ->
+    | (Endpoint _ | Intent | UserControlledString | UserControlledURI), CreateIntent ->
         (* creating Intent from user-congrolled data *)
         Some IssueType.untrusted_intent_creation
-    | (Intent | IntentFromURI | UserControlledString | UserControlledURI), CreateFile ->
+    | (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI), CreateFile ->
         (* user-controlled file creation; may be vulnerable to path traversal + more *)
         Some IssueType.untrusted_file
-    | (Intent | IntentFromURI | UserControlledString | UserControlledURI), Deserialization ->
+    | ( (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI)
+      , Deserialization ) ->
         (* shouldn't let anyone external control what we deserialize *)
         Some IssueType.untrusted_deserialization
-    | (Intent | IntentFromURI | UserControlledString | UserControlledURI), HTML ->
+    | (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI), HTML ->
         (* untrusted data flows into HTML; XSS risk *)
         Some IssueType.cross_site_scripting
-    | (Intent | IntentFromURI | UserControlledString | UserControlledURI), JavaScript ->
+    | (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI), JavaScript ->
         (* untrusted data flows into JS *)
         Some IssueType.javascript_injection
     | DrawableResource _, OpenDrawableResource ->
@@ -481,7 +509,7 @@ include Trace.Make (struct
         Some IssueType.create_intent_from_uri
     | PrivateData, Logging ->
         Some IssueType.logging_private_data
-    | (Intent | UserControlledString | UserControlledURI), ShellExec ->
+    | (Endpoint _ | Intent | UserControlledString | UserControlledURI), ShellExec ->
         Some IssueType.shell_injection
     | Other, _ | _, Other ->
         (* for testing purposes, Other matches everything *)

infer/tests/codetoanalyze/java/quandary/Services.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2018 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package codetoanalyze.java.quandary;
+
+import java.io.IOException;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.Runtime;
+
+class Services {
+
+}
+
+// classes annotated with @ThriftService are servers (sources), whereas interfaces
+// annotated with @ThriftService are clients (sinks): see
+// https://github.com/facebook/swift/blob/master/swift-service/README.md#clients-and-servers
+@Retention(RetentionPolicy.CLASS)
+@interface ThriftService {
+}
+
+
+@ThriftService
+class Service1 {
+
+  public void serviceMethodBad(String s) throws IOException {
+    Runtime.getRuntime().exec(s); // RCE if s is tainted, we should warn
+  }
+
+}
+
+@ThriftService
+interface ThriftInterface {
+
+  public void interfaceServiceMethodBad(String s) throws IOException;
+}
+
+// this is a service too
+class Implementer implements ThriftInterface {
+
+  public void interfaceServiceMethodBad(String s) throws IOException {
+    Runtime.getRuntime().exec(s); // RCE if s is tainted, we should warn
+  }
+
+}

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

@@ -174,6 +174,8 @@ codetoanalyze/java/quandary/LoggingPrivateData.java, void LoggingPrivateData.log
 codetoanalyze/java/quandary/Recursion.java, void Recursion.callSinkThenDivergeBad(), 1, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void Recursion.callSinkThenDiverge(Object) with tainted index 0,Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]
 codetoanalyze/java/quandary/Recursion.java, void Recursion.safeRecursionCallSinkBad(), 1, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void Recursion.safeRecursionCallSink(int,Object) with tainted index 0,Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]
 codetoanalyze/java/quandary/Serialization.java, Object Serialization.taintedObjectInputStreamBad(), 2, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to ObjectInputStream.<init>(InputStream) with tainted index 1]
+codetoanalyze/java/quandary/Services.java, void Implementer.interfaceServiceMethodBad(String), 1, SHELL_INJECTION, [Return from void Implementer.interfaceServiceMethodBad(String),Call to Process Runtime.exec(String) with tainted index 1]
+codetoanalyze/java/quandary/Services.java, void Service1.serviceMethodBad(String), 1, SHELL_INJECTION, [Return from void Service1.serviceMethodBad(String),Call to Process Runtime.exec(String) with tainted index 1]
 codetoanalyze/java/quandary/Strings.java, void Strings.viaFormatterBad(), 3, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]
 codetoanalyze/java/quandary/Strings.java, void Strings.viaFormatterIgnoreReturnBad(), 4, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]
 codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBufferBad(), 3, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]