From 174bdcd22b17428100ed9b0d3908f4821530717c Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Wed, 7 Nov 2018 04:04:17 -0800 Subject: [PATCH] [quandary] Add class-loading sinks Reviewed By: jeremydubreil Differential Revision: D12942819 fbshipit-source-id: c294b4238 --- infer/src/quandary/JavaTrace.ml | 13 ++++++- .../java/quandary/ClassLoading.java | 39 +++++++++++++++++++ .../codetoanalyze/java/quandary/issues.exp | 2 + 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/quandary/ClassLoading.java diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index 62ce62fd9..5898784f8 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -261,6 +261,7 @@ module JavaSource = Source.Make (SourceKind) module SinkKind = struct type t = + | ClassLoading | CreateFile (** sink that creates a file *) | CreateIntent (** sink that creates an Intent *) | OpenDrawableResource (** sink that inflates a Drawable resource from an integer ID *) @@ -279,6 +280,8 @@ module SinkKind = struct let matches ~caller ~callee = Int.equal 0 (compare caller callee) let of_string = function + | "ClassLoading" -> + ClassLoading | "CreateFile" -> CreateFile | "CreateIntent" -> @@ -417,6 +420,10 @@ module SinkKind = struct taint_all CreateFile | "java.io.ObjectInputStream", "" -> taint_all Deserialization + | "java.lang.Class", "forName" | "java.lang.ClassLoader", "loadClass" -> + taint_nth 0 ClassLoading + | "java.lang.ClassLoader", "defineClass" -> + taint_nth 1 ClassLoading | "java.lang.ProcessBuilder", "" -> taint_all ShellExec | "java.lang.ProcessBuilder", "command" -> @@ -444,6 +451,8 @@ module SinkKind = struct let pp fmt kind = F.fprintf fmt ( match kind with + | ClassLoading -> + "ClassLoading" | CreateFile -> "CreateFile" | CreateIntent -> @@ -548,9 +557,9 @@ include Trace.Make (struct Some IssueType.create_intent_from_uri | PrivateData, Logging -> Some IssueType.logging_private_data - | (Intent | UserControlledString | UserControlledURI), ShellExec -> + | (Intent | UserControlledString | UserControlledURI), (ClassLoading | ShellExec) -> Some IssueType.shell_injection - | Endpoint _, ShellExec -> + | Endpoint _, (ClassLoading | ShellExec) -> Some IssueType.shell_injection_risk | Other, _ | _, Other -> (* for testing purposes, Other matches everything *) diff --git a/infer/tests/codetoanalyze/java/quandary/ClassLoading.java b/infer/tests/codetoanalyze/java/quandary/ClassLoading.java new file mode 100644 index 000000000..1079e1f11 --- /dev/null +++ b/infer/tests/codetoanalyze/java/quandary/ClassLoading.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2018-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package codetoanalyze.java.quandary; + +import android.content.ClipboardManager; + +public class ClassLoading { + ClipboardManager clipboard; + + public String getUserControlledString() { + return this.clipboard.getText().toString(); + } + + public void clipboardToClassForNameBad() { + try { + Class cls = Class.forName(this.getUserControlledString()); + } catch (Exception e) { + System.out.println("Exception: " + e); + } + } + + /* + We don't want to report it as we consider that string concatenation + sanitizes the user-controlled string for class loading. + */ + public void FP_clipboardToClassForNameWithConcatenationGood() { + String javaFileName = "blabla." + this.getUserControlledString(); + try { + Class cls = Class.forName(javaFileName); + } catch (Exception e) { + System.out.println("Exception: " + e); + } + } +} diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 3eba0d9ff..ff3659320 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -26,6 +26,8 @@ codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.viaV codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.viaVarBad3():void, 4, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0] codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.whileBad1(int):void, 3, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0] codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.whileBad2(int):void, 6, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0] +codetoanalyze/java/quandary/ClassLoading.java, codetoanalyze.java.quandary.ClassLoading.FP_clipboardToClassForNameWithConcatenationGood():void, 3, SHELL_INJECTION, no_bucket, ERROR, [Return from CharSequence ClipboardManager.getText() with tainted data return*,Return from String ClassLoading.getUserControlledString(),Call to Class Class.forName(String) with tainted index 0] +codetoanalyze/java/quandary/ClassLoading.java, codetoanalyze.java.quandary.ClassLoading.clipboardToClassForNameBad():void, 2, SHELL_INJECTION, no_bucket, ERROR, [Return from CharSequence ClipboardManager.getText() with tainted data return*,Return from String ClassLoading.getUserControlledString(),Call to Class Class.forName(String) with tainted index 0] codetoanalyze/java/quandary/ContentProviders.java, codetoanalyze.java.quandary.ContentProviders.bulkInsert(android.net.Uri,android.content.ContentValues[]):int, 1, UNTRUSTED_FILE, no_bucket, ERROR, [Return from int ContentProviders.bulkInsert(Uri,android.content.ContentValues[]),Call to File.(String) with tainted index 1] codetoanalyze/java/quandary/ContentProviders.java, codetoanalyze.java.quandary.ContentProviders.call(java.lang.String,java.lang.String,android.os.Bundle):android.os.Bundle, 1, UNTRUSTED_FILE, no_bucket, ERROR, [Return from Bundle ContentProviders.call(String,String,Bundle),Call to File.(String) with tainted index 1] codetoanalyze/java/quandary/ContentProviders.java, codetoanalyze.java.quandary.ContentProviders.delete(android.net.Uri,java.lang.String,java.lang.String[]):int, 1, UNTRUSTED_FILE, no_bucket, ERROR, [Return from int ContentProviders.delete(Uri,String,java.lang.String[]),Call to File.(String) with tainted index 1]