From 30d7239aff6a0e7ec8a07c348deffc79909b4096 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Fri, 27 Apr 2018 16:33:04 -0700 Subject: [PATCH] [quandary] SQL sinks for java Reviewed By: jeremydubreil Differential Revision: D7799801 fbshipit-source-id: 002079d --- infer/src/quandary/JavaTrace.ml | 30 +++++++++++++++++++ .../codetoanalyze/java/quandary/Services.java | 25 ++++++++++++++++ .../codetoanalyze/java/quandary/issues.exp | 5 ++++ 3 files changed, 60 insertions(+) diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index 70fa21df9..a1b40819d 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -271,6 +271,9 @@ module SinkKind = struct | JavaScript (** sink that passes its arguments to untrusted JS code *) | Logging (** sink that logs one or more of its arguments *) | ShellExec (** sink that runs a shell command *) + | SQLInjection (** unescaped query to a SQL database (could be a read or a write) *) + | SQLRead (** escaped read to a SQL database *) + | SQLWrite (** escaped write to a SQL database *) | StartComponent (** sink that launches an Activity, Service, etc. *) | Other (** for testing or uncategorized sinks *) [@@deriving compare] @@ -294,6 +297,12 @@ module SinkKind = struct OpenDrawableResource | "ShellExec" -> ShellExec + | "SQLInjection" -> + SQLInjection + | "SQLRead" -> + SQLRead + | "SQLWrite" -> + SQLWrite | "StartComponent" -> StartComponent | _ -> @@ -411,6 +420,13 @@ module SinkKind = struct taint_all ShellExec | "java.lang.Runtime", "exec" -> taint_nth 0 ShellExec + (* TODO: separate non-injection sinks for PreparedStatement's *) + | "java.sql.Statement", ("addBatch" | "execute") -> + taint_nth 0 SQLInjection + | "java.sql.Statement", "executeQuery" -> + taint_nth 0 SQLRead + | "java.sql.Statement", ("executeUpdate" | "executeLargeUpdate") -> + taint_nth 0 SQLWrite | class_name, method_name -> get_external_sink class_name method_name in @@ -441,6 +457,12 @@ module SinkKind = struct "OpenDrawableResource" | ShellExec -> "ShellExec" + | SQLInjection -> + "SQLInjection" + | SQLRead -> + "SQLRead" + | SQLWrite -> + "SQLWrite" | StartComponent -> "StartComponent" | Other -> @@ -505,6 +527,14 @@ include Trace.Make (struct | (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI), JavaScript -> (* untrusted data flows into JS *) Some IssueType.javascript_injection + | ( (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI) + , SQLInjection ) -> + (* untrusted and unescaped data flows to SQL *) + Some IssueType.sql_injection_risk + | ( (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI) + , (SQLRead | SQLWrite) ) -> + (* untrusted data flows to SQL *) + Some IssueType.user_controlled_sql_risk | DrawableResource _, OpenDrawableResource -> (* not a security issue, but useful for debugging flows from resource IDs to inflation *) Some IssueType.quandary_taint_error diff --git a/infer/tests/codetoanalyze/java/quandary/Services.java b/infer/tests/codetoanalyze/java/quandary/Services.java index a3ddc4314..ae2de6916 100644 --- a/infer/tests/codetoanalyze/java/quandary/Services.java +++ b/infer/tests/codetoanalyze/java/quandary/Services.java @@ -13,6 +13,8 @@ import java.io.IOException; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.Runtime; +import java.sql.Statement; +import java.sql.SQLException; class Services { @@ -33,6 +35,29 @@ class Service1 { Runtime.getRuntime().exec(s); // RCE if s is tainted, we should warn } + Statement mStatement; + + public void paramToSql1Bad(String s) throws SQLException { + mStatement.execute(s); + } + + public void paramToSql2Bad(String s) throws SQLException { + mStatement.executeLargeUpdate(s); + } + + public void paramToSql3Bad(String s) throws SQLException { + mStatement.executeQuery(s); + } + + public void paramToSql4Bad(String s) throws SQLException { + mStatement.executeUpdate(s); + } + + public void paramToSql5Bad(String s) throws SQLException { + mStatement.addBatch(s); + mStatement.executeBatch(); + } + // assume protected methods aren't exported to Thrift protected void protectedServiceMethodOk(String s) throws IOException { Runtime.getRuntime().exec(s); diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index d5888531f..0e624d348 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -177,6 +177,11 @@ codetoanalyze/java/quandary/Recursion.java, void Recursion.callSinkThenDivergeBa codetoanalyze/java/quandary/Recursion.java, void Recursion.safeRecursionCallSinkBad(), 1, QUANDARY_TAINT_ERROR, 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, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to ObjectInputStream.(InputStream) with tainted index 1] codetoanalyze/java/quandary/Services.java, void Implementer.interfaceServiceMethodBad(String), 1, SHELL_INJECTION_RISK, ERROR, [Return from void Implementer.interfaceServiceMethodBad(String),Call to Process Runtime.exec(String) with tainted index 1] +codetoanalyze/java/quandary/Services.java, void Service1.paramToSql1Bad(String), 1, SQL_INJECTION_RISK, ERROR, [Return from void Service1.paramToSql1Bad(String),Call to boolean Statement.execute(String) with tainted index 1] +codetoanalyze/java/quandary/Services.java, void Service1.paramToSql2Bad(String), 1, USER_CONTROLLED_SQL_RISK, ERROR, [Return from void Service1.paramToSql2Bad(String),Call to long Statement.executeLargeUpdate(String) with tainted index 1] +codetoanalyze/java/quandary/Services.java, void Service1.paramToSql3Bad(String), 1, USER_CONTROLLED_SQL_RISK, ERROR, [Return from void Service1.paramToSql3Bad(String),Call to ResultSet Statement.executeQuery(String) with tainted index 1] +codetoanalyze/java/quandary/Services.java, void Service1.paramToSql4Bad(String), 1, USER_CONTROLLED_SQL_RISK, ERROR, [Return from void Service1.paramToSql4Bad(String),Call to int Statement.executeUpdate(String) with tainted index 1] +codetoanalyze/java/quandary/Services.java, void Service1.paramToSql5Bad(String), 1, SQL_INJECTION_RISK, ERROR, [Return from void Service1.paramToSql5Bad(String),Call to void Statement.addBatch(String) with tainted index 1] codetoanalyze/java/quandary/Services.java, void Service1.serviceMethodBad(String), 1, SHELL_INJECTION_RISK, ERROR, [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, 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, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]