Merge "Add content provider permission check"

5 files changed, 160 insertions, 2 deletions

diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
index c9abddb..a2757cb 100644
--- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
+++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java
@@ -56,6 +56,7 @@ public class LintConstants {
     public static final String TAG_RECEIVER = "receiver";              //$NON-NLS-1$
     public static final String TAG_PROVIDER = "provider";              //$NON-NLS-1$
     public static final String TAG_GRANT_PERMISSION = "grant-uri-permission"; //$NON-NLS-1$
+    public static final String TAG_PATH_PERMISSION = "path-permission"; //$NON-NLS-1$
 
     // Tags: Resources
     public static final String TAG_RESOURCES = "resources";            //$NON-NLS-1$
@@ -129,6 +130,8 @@ public class LintConstants {
     public static final String ATTR_PATH_PREFIX = "pathPrefix";        //$NON-NLS-1$
     public static final String ATTR_PATH_PATTERN = "pathPattern";      //$NON-NLS-1$
     public final static String ATTR_DEBUGGABLE = "debuggable";         //$NON-NLS-1$
+    public final static String ATTR_READ_PERMISSION = "readPermission"; //$NON_NLS-1$
+    public final static String ATTR_WRITE_PERMISSION = "writePermission"; //$NON_NLS-1$
 
     // Attributes: Resources
     public static final String ATTR_NAME = "name";                     //$NON-NLS-1$
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecurityDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecurityDetector.java
index c43fba3..13b5c83 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecurityDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecurityDetector.java
@@ -23,9 +23,13 @@ import static com.android.tools.lint.detector.api.LintConstants.ATTR_PATH;
 import static com.android.tools.lint.detector.api.LintConstants.ATTR_PATH_PATTERN;
 import static com.android.tools.lint.detector.api.LintConstants.ATTR_PATH_PREFIX;
 import static com.android.tools.lint.detector.api.LintConstants.ATTR_PERMISSION;
+import static com.android.tools.lint.detector.api.LintConstants.ATTR_READ_PERMISSION;
+import static com.android.tools.lint.detector.api.LintConstants.ATTR_WRITE_PERMISSION;
 import static com.android.tools.lint.detector.api.LintConstants.TAG_APPLICATION;
 import static com.android.tools.lint.detector.api.LintConstants.TAG_GRANT_PERMISSION;
 import static com.android.tools.lint.detector.api.LintConstants.TAG_INTENT_FILTER;
+import static com.android.tools.lint.detector.api.LintConstants.TAG_PATH_PERMISSION;
+import static com.android.tools.lint.detector.api.LintConstants.TAG_PROVIDER;
 import static com.android.tools.lint.detector.api.LintConstants.TAG_SERVICE;
 
 import com.android.tools.lint.detector.api.Category;
@@ -48,7 +52,6 @@ import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
@@ -80,6 +83,21 @@ public class SecurityDetector extends Detector implements Detector.XmlScanner,
             SecurityDetector.class,
             EnumSet.of(Scope.MANIFEST));
 
+    /** Exported content providers */
+    public static final Issue EXPORTED_PROVIDER = Issue.create(
+            "ExportedContentProvider", //$NON-NLS-1$
+            "Checks for exported content providers that do not require permissions",
+            "Content providers are exported by default and any application on the " +
+            "system can potentially use them to read and write data. If the content" +
+            "provider provides access to sensitive data, it should be protected by " +
+            "specifying export=false in the manifest or by protecting it with a " +
+            "permission that can be granted to other applications.",
+            Category.SECURITY,
+            5,
+            Severity.WARNING,
+            SecurityDetector.class,
+            EnumSet.of(Scope.MANIFEST));
+
     /** Content provides which grant all URIs access */
     public static final Issue OPEN_PROVIDER = Issue.create(
             "GrantAllUris", //$NON-NLS-1$
@@ -144,7 +162,8 @@ public class SecurityDetector extends Detector implements Detector.XmlScanner,
     public Collection<String> getApplicableElements() {
         return Arrays.asList(
             TAG_SERVICE,
-            TAG_GRANT_PERMISSION
+            TAG_GRANT_PERMISSION,
+            TAG_PROVIDER
         );
     }
 
@@ -155,6 +174,8 @@ public class SecurityDetector extends Detector implements Detector.XmlScanner,
             checkService(context, element);
         } else if (tag.equals(TAG_GRANT_PERMISSION)) {
             checkGrantPermission(context, element);
+        } else if (tag.equals(TAG_PROVIDER)) {
+            checkProvider(context, element);
         }
     }
 
@@ -212,6 +233,49 @@ public class SecurityDetector extends Detector implements Detector.XmlScanner,
         }
     }
 
+    private void checkProvider(XmlContext context, Element element) {
+        String exportValue = element.getAttributeNS(ANDROID_URI, ATTR_EXPORTED);
+        // Content providers are exported by default
+        boolean exported = true;
+        if (exportValue != null && exportValue.length() > 0) {
+            exported = Boolean.valueOf(exportValue);
+        }
+
+        if (exported) {
+            // Just check for some use of permissions. Other Lint checks can check the saneness
+            // of the permissions. We'll accept the permission, readPermission, or writePermission
+            // attributes on the provider element, or a path-permission element.
+            String permission = element.getAttributeNS(ANDROID_URI, ATTR_READ_PERMISSION);
+            if (permission == null || permission.length() == 0) {
+                permission = element.getAttributeNS(ANDROID_URI, ATTR_WRITE_PERMISSION);
+                if (permission == null || permission.length() == 0) {
+                    permission = element.getAttributeNS(ANDROID_URI, ATTR_PERMISSION);
+                    if (permission == null || permission.length() == 0) {
+                        // No permission attributes? Check for path-permission.
+
+                        // TODO: Add a Lint check to ensure the path-permission is good, similar to
+                        // the grant-uri-permission check.
+                        boolean hasPermission = false;
+                        for (Element child : LintUtils.getChildren(element)) {
+                            String tag = child.getTagName();
+                            if (tag.equals(TAG_PATH_PERMISSION)) {
+                                hasPermission = true;
+                                break;
+                            }
+                        }
+
+                        if (!hasPermission) {
+                            context.report(EXPORTED_PROVIDER, element, context.getLocation(element),
+                                    "Exported content providers can provide access to " +
+                                            "potentially sensitive data",
+                                    null);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     // ---- Implements Detector.JavaScanner ----
 
     @Override
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SecurityDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SecurityDetectorTest.java
index 248273a..a23b297 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SecurityDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SecurityDetectorTest.java
@@ -78,6 +78,25 @@ public class SecurityDetectorTest extends AbstractCheckTest {
                     "res/values/strings.xml"));
     }
 
+    // exportprovider1.xml has two exported content providers with no permissions
+    public void testContentProvider1() throws Exception {
+        assertEquals(
+                "AndroidManifest.xml:14: Warning: Exported content providers can provide access to potentially sensitive data\n" +
+                "AndroidManifest.xml:20: Warning: Exported content providers can provide access to potentially sensitive data",
+                 lintProject(
+                        "exportprovider1.xml=>AndroidManifest.xml",
+                        "res/values/strings.xml"));
+    }
+
+    // exportprovider2.xml has no un-permissioned exported content providers
+    public void testContentProvider2() throws Exception {
+        assertEquals(
+                "No warnings.",
+                lintProject(
+                        "exportprovider2.xml=>AndroidManifest.xml",
+                        "res/values/strings.xml"));
+    }
+
     public void testWorldWriteable() throws Exception {
         assertEquals(
             "WorldWriteableFile.java:25: Warning: Using MODE_WORLD_WRITEABLE when creating files can be risky, review carefully\n" +
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/exportprovider1.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/exportprovider1.xml
new file mode 100644
index 0000000..02ec2e0
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/exportprovider1.xml
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="foo.bar2"
+    android:versionCode="1"
+    android:versionName="1.0" >
+
+    <uses-sdk android:minSdkVersion="14" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name" >
+
+        <!-- exported implicitly, fail -->
+        <provider
+            android:name="com.sample.provider.providerClass1"
+            android:authorities="com.sample.provider.providerData">
+        </provider>
+
+        <!-- exported explicitly, fail -->
+        <provider
+            android:exported="true"
+            android:name="com.sample.provider.providerClass2"
+            android:authorities="com.sample.provider.providerData">
+        </provider>
+
+        <!-- not exported, win -->
+        <provider
+            android:exported="false"
+            android:name="com.sample.provider.providerClass3"
+            android:authorities="com.sample.provider.providerData">
+        </provider>
+    </application>
+</manifest>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/exportprovider2.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/exportprovider2.xml
new file mode 100644
index 0000000..4e3fc1b
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/exportprovider2.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="foo.bar2"
+    android:versionCode="1"
+    android:versionName="1.0" >
+
+    <uses-sdk android:minSdkVersion="14" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name" >
+
+        <!-- read+write permission attribute, win -->
+        <provider
+            android:name="com.sample.provider.providerClass"
+            android:authorities="com.sample.provider.providerData"
+            android:readPermission="com.sample.provider.READ_PERMISSON"
+            android:writePermission="com.sample.provider.WRITE_PERMISSON">
+        </provider>
+
+        <!-- permission attribute, win -->
+        <provider
+            android:name="com.sample.provider.providerClass"
+            android:authorities="com.sample.provider.providerData"
+            android:permission="com.sample.provider.PERMISSION">
+        </provider>
+
+        <!-- path-permission, win -->
+        <provider
+            android:name="com.sample.provider.providerClass"
+            android:authorities="com.sample.provider.providerData">
+            <path-permission
+                android:pathPrefix="/hello"
+                android:permission="com.sample.provider.PERMISSION">
+            </path-permission>
+        </provider>
+    </application>
+
+</manifest>