From 99f79587cd655e5545f13b56c97c7bd79e5dd55a Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Wed, 9 Dec 2015 17:07:45 -0800 Subject: [PATCH] adding ContentValues as sink Reviewed By: jeremydubreil Differential Revision: D2735179 fb-gh-sync-id: a9e4d59 --- .../src/android/content/ContentValues.java | 38 +++++++++++++++++++ infer/tests/ant_report.json | 5 +++ infer/tests/buck_report.json | 5 +++ .../java/infer/TaintExample.java | 12 ++++++ .../tests/endtoend/java/infer/TaintTest.java | 3 +- 5 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 infer/models/java/src/android/content/ContentValues.java diff --git a/infer/models/java/src/android/content/ContentValues.java b/infer/models/java/src/android/content/ContentValues.java new file mode 100644 index 000000000..b94f38c9d --- /dev/null +++ b/infer/models/java/src/android/content/ContentValues.java @@ -0,0 +1,38 @@ +/* +* Copyright (c) 2013 - 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 android.content; + +import com.facebook.infer.models.InferBuiltins; + +public class ContentValues { + + /** + * We want to treat this as a sink for both privacy and security purposes. + * + * Privacy: The purpose of ContentValues is to feed information into a ContentProvider, a core + * Android component that can expose data for other Android applications to query. Thus, you do + * not want secret information to flow into ContentValues because you don't want to give other + * apps an interface that lets them query your secret information. There's a possibility for false + * positives here because ContentProviders can control access to information via permissions, but + * in general it's just a bad idea to let secret info into a ContentProvider. + * + * Security: You don't want untrusted external content to flow into ContentValues because it may + * be used to store data in a ContentProvider, potentially giving an external app control over + * what goes into the database backing the ContentProvider/giving them direct access to make + * queries on your content provider (rather than exposing a small set of trusted queries, which is + * what should be done). This could have any number of security implications depending on how the + * ContentProvider is used. + **/ + public void put(String key, String value) { + InferBuiltins.__check_untainted(key); + InferBuiltins.__check_untainted(value); + } + +} diff --git a/infer/tests/ant_report.json b/infer/tests/ant_report.json index 062364c72..b2bd8a3bf 100644 --- a/infer/tests/ant_report.json +++ b/infer/tests/ant_report.json @@ -4,6 +4,11 @@ "file": "codetoanalyze/java/infer/TaintExample.java", "procedure": "Socket TaintExample.callReadInputStreamCauseTaintError(SSLSocketFactory)" }, + { + "bug_type": "TAINTED_VALUE_REACHING_SENSITIVE_FUNCTION", + "file": "codetoanalyze/java/infer/TaintExample.java", + "procedure": "void TaintExample.contentValuesPutWithTaintedString(ContentValues,SharedPreferences,String,String)" + }, { "bug_type": "CONTEXT_LEAK", "file": "codetoanalyze/java/infer/ContextLeaks.java", diff --git a/infer/tests/buck_report.json b/infer/tests/buck_report.json index 4034916a3..c1347dc41 100644 --- a/infer/tests/buck_report.json +++ b/infer/tests/buck_report.json @@ -4,6 +4,11 @@ "file": "infer/tests/codetoanalyze/java/infer/TaintExample.java", "procedure": "Socket TaintExample.callReadInputStreamCauseTaintError(SSLSocketFactory)" }, + { + "bug_type": "TAINTED_VALUE_REACHING_SENSITIVE_FUNCTION", + "file": "infer/tests/codetoanalyze/java/infer/TaintExample.java", + "procedure": "void TaintExample.contentValuesPutWithTaintedString(ContentValues,SharedPreferences,String,String)" + }, { "bug_type": "CONTEXT_LEAK", "file": "infer/tests/codetoanalyze/java/infer/ContextLeaks.java", diff --git a/infer/tests/codetoanalyze/java/infer/TaintExample.java b/infer/tests/codetoanalyze/java/infer/TaintExample.java index 915770417..08dc7b7b3 100644 --- a/infer/tests/codetoanalyze/java/infer/TaintExample.java +++ b/infer/tests/codetoanalyze/java/infer/TaintExample.java @@ -21,6 +21,9 @@ import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; +import android.content.ContentValues; +import android.content.SharedPreferences; + import com.facebook.infer.models.InferTaint; public class TaintExample { @@ -158,4 +161,13 @@ public class TaintExample { callSinkMethodModelMethodsUndefined(returnTaintedSourceModelMethodsUndefined()); } + public void contentValuesPutWithTaintedString(ContentValues values, SharedPreferences prefs, + String key, String value) { + values.put(key, prefs.getString(key, value)); + } + + public void contentValuesPutOk(ContentValues values, String key, String value) { + values.put(key, value); + } + } diff --git a/infer/tests/endtoend/java/infer/TaintTest.java b/infer/tests/endtoend/java/infer/TaintTest.java index 83fea78f5..d7d394e1a 100644 --- a/infer/tests/endtoend/java/infer/TaintTest.java +++ b/infer/tests/endtoend/java/infer/TaintTest.java @@ -53,7 +53,8 @@ public class TaintTest { "simpleTaintErrorWithModelMethodsUndefined", "interprocTaintErrorWithModelMethodsUndefined1", "interprocTaintErrorWithModelMethodsUndefined2", - "interprocTaintErrorWithModelMethodsUndefined3" + "interprocTaintErrorWithModelMethodsUndefined3", + "contentValuesPutWithTaintedString" }; assertThat(