Merge "Add allowBackup lint security check"

8 files changed, 137 insertions, 3 deletions

diff --git a/common/src/com/android/SdkConstants.java b/common/src/com/android/SdkConstants.java
index cf64967..8c798c8 100644
--- a/common/src/com/android/SdkConstants.java
+++ b/common/src/com/android/SdkConstants.java
@@ -683,6 +683,7 @@ public final class SdkConstants {
     public static final String ATTR_PATH = "path";                     //$NON-NLS-1$
     public static final String ATTR_PATH_PREFIX = "pathPrefix";        //$NON-NLS-1$
     public static final String ATTR_PATH_PATTERN = "pathPattern";      //$NON-NLS-1$
+    public static final String ATTR_ALLOW_BACKUP = "allowBackup";      //$NON_NLS-1$
     public static final String ATTR_DEBUGGABLE = "debuggable";         //$NON-NLS-1$
     public static final String ATTR_READ_PERMISSION = "readPermission"; //$NON_NLS-1$
     public static final String ATTR_WRITE_PERMISSION = "writePermission"; //$NON_NLS-1$
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/LintFix.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/LintFix.java
index c02c303..7683f8d 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/LintFix.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/LintFix.java
@@ -23,6 +23,7 @@ import com.android.tools.lint.checks.AccessibilityDetector;
 import com.android.tools.lint.checks.DetectMissingPrefix;
 import com.android.tools.lint.checks.HardcodedValuesDetector;
 import com.android.tools.lint.checks.InefficientWeightDetector;
+import com.android.tools.lint.checks.ManifestOrderDetector;
 import com.android.tools.lint.checks.ObsoleteLayoutParamsDetector;
 import com.android.tools.lint.checks.PxUsageDetector;
 import com.android.tools.lint.checks.ScrollViewChildDetector;
@@ -148,6 +149,7 @@ abstract class LintFix implements ICompletionProposal {
                 LinearLayoutWeightFix.class);
         sFixes.put(AccessibilityDetector.ISSUE.getId(), SetAttributeFix.class);
         sFixes.put(InefficientWeightDetector.BASELINE_WEIGHTS.getId(), SetAttributeFix.class);
+        sFixes.put(ManifestOrderDetector.ALLOW_BACKUP.getId(), SetAttributeFix.class);
         sFixes.put(HardcodedValuesDetector.ISSUE.getId(), ExtractStringFix.class);
         sFixes.put(UselessViewDetector.USELESS_LEAF.getId(), RemoveUselessViewFix.class);
         sFixes.put(UselessViewDetector.USELESS_PARENT.getId(), RemoveUselessViewFix.class);
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/SetAttributeFix.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/SetAttributeFix.java
index 4ed5e05..1d743b8 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/SetAttributeFix.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/SetAttributeFix.java
@@ -15,6 +15,7 @@
  */
 package com.android.ide.eclipse.adt.internal.lint;
 
+import static com.android.SdkConstants.ATTR_ALLOW_BACKUP;
 import static com.android.SdkConstants.ATTR_BASELINE_ALIGNED;
 import static com.android.SdkConstants.ATTR_CONTENT_DESCRIPTION;
 import static com.android.SdkConstants.ATTR_INPUT_TYPE;
@@ -24,6 +25,7 @@ import static com.android.SdkConstants.VALUE_FALSE;
 
 import com.android.tools.lint.checks.AccessibilityDetector;
 import com.android.tools.lint.checks.InefficientWeightDetector;
+import com.android.tools.lint.checks.ManifestOrderDetector;
 import com.android.tools.lint.checks.SecurityDetector;
 import com.android.tools.lint.checks.TextFieldDetector;
 import com.android.tools.lint.checks.TranslationDetector;
@@ -48,6 +50,8 @@ final class SetAttributeFix extends SetPropertyFix {
             return ATTR_INPUT_TYPE;
         } else if (mId.equals(TranslationDetector.MISSING.getId())) {
             return ATTR_TRANSLATABLE;
+        } else if (mId.equals(ManifestOrderDetector.ALLOW_BACKUP.getId())) {
+            return ATTR_ALLOW_BACKUP;
         } else {
             assert false : mId;
             return "";
@@ -75,6 +79,8 @@ final class SetAttributeFix extends SetPropertyFix {
             return "Add permission attribute";
         } else if (mId.equals(TranslationDetector.MISSING.getId())) {
             return "Mark this as a non-translatable resource";
+        } else if (mId.equals(ManifestOrderDetector.ALLOW_BACKUP.getId())) {
+            return "Set the allowBackup attribute to true or false";
         } else {
             assert false : mId;
             return "";
@@ -95,7 +101,8 @@ final class SetAttributeFix extends SetPropertyFix {
     @Override
     protected boolean invokeCodeCompletion() {
         return mId.equals(SecurityDetector.EXPORTED_SERVICE.getId())
-                || mId.equals(TextFieldDetector.ISSUE.getId());
+                || mId.equals(TextFieldDetector.ISSUE.getId())
+                || mId.equals(ManifestOrderDetector.ALLOW_BACKUP.getId());
     }
 
     @Override
@@ -117,5 +124,4 @@ final class SetAttributeFix extends SetPropertyFix {
 
         return super.getProposal();
     }
-
 }
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
index c5e4e64..476ebf9 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
@@ -55,7 +55,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
     private static final List<Issue> sIssues;
 
     static {
-        final int initialCapacity = 110;
+        final int initialCapacity = 111;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -118,6 +118,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(ManifestOrderDetector.WRONG_PARENT);
         issues.add(ManifestOrderDetector.DUPLICATE_ACTIVITY);
         issues.add(ManifestOrderDetector.TARGET_NEWER);
+        issues.add(ManifestOrderDetector.ALLOW_BACKUP);
         issues.add(SecurityDetector.EXPORTED_PROVIDER);
         issues.add(SecurityDetector.EXPORTED_SERVICE);
         issues.add(SecurityDetector.EXPORTED_ACTIVITY);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
index 237d8a0..c95b12d 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
@@ -31,6 +31,7 @@ import static com.android.SdkConstants.TAG_USES_LIBRARY;
 import static com.android.SdkConstants.TAG_USES_PERMISSION;
 import static com.android.SdkConstants.TAG_USES_SDK;
 
+import com.android.SdkConstants;
 import com.android.annotations.NonNull;
 import com.android.tools.lint.detector.api.Category;
 import com.android.tools.lint.detector.api.Context;
@@ -168,6 +169,40 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
             ManifestOrderDetector.class,
             EnumSet.of(Scope.MANIFEST));
 
+    /** Not explicitly defining allowBackup */
+    public static final Issue ALLOW_BACKUP = Issue.create(
+            "AllowBackup", //$NON-NLS-1$
+            "Ensure that allowBackup is explicitly set in the application's manifest",
+
+            "The allowBackup attribute determines if an application's data can be backed up " +
+            "and restored. It is documented at " +
+            "http://developer.android.com/reference/android/R.attr.html#allowBackup\n" +
+            "\n" +
+            "By default, this flag is set to `true`. When this flag is set to `true`, " +
+            "application data can be backed up and restored by the user using `adb backup` " +
+            "and `adb restore`.\n" +
+            "\n" +
+            "This may have security consequences for an application. `adb backup` allows " +
+            "users who have enabled USB debugging to copy application data off of the " +
+            "device. Once backed up, all application data can be read by the user. " +
+            "`adb restore` allows creation of application data from a source specified " +
+            "by the user. Following a restore, applications should not assume that the " +
+            "data, file permissions, and directory permissions were created by the " +
+            "application itself.\n" +
+            "\n" +
+            "Setting `allowBackup=\"false\"` opts an application out of both backup and " +
+            "restore.\n" +
+            "\n" +
+            "To fix this warning, decide whether your application should support backup, " +
+            "and explicitly set `android:allowBackup=(true|false)\"`",
+
+            Category.SECURITY,
+            3,
+            Severity.WARNING,
+            ManifestOrderDetector.class,
+            EnumSet.of(Scope.MANIFEST)).setMoreInfo(
+                    "http://developer.android.com/reference/android/R.attr.html#allowBackup");
+
     /** Constructs a new {@link ManifestOrderDetector} check */
     public ManifestOrderDetector() {
     }
@@ -347,6 +382,13 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
 
         if (tag.equals(TAG_APPLICATION)) {
             mSeenApplication = true;
+            if (!element.hasAttributeNS(ANDROID_URI, SdkConstants.ATTR_ALLOW_BACKUP)
+                    && context.isEnabled(ALLOW_BACKUP)) {
+                context.report(ALLOW_BACKUP, element, context.getLocation(element),
+                        String.format("Should explicitly set android:allowBackup to true or " +
+                            "false (it's true by default, and that can have some security " +
+                            "implications for the application's data)", tag), null);
+            }
         } else if (mSeenApplication) {
             if (context.isEnabled(ORDER)) {
                 context.report(ORDER, element, context.getLocation(element),
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java
index 7513138..8d49bfa 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java
@@ -199,4 +199,37 @@ public class ManifestOrderDetectorTest extends AbstractCheckTest {
                     "duplicate-manifest-ignore.xml=>AndroidManifest.xml",
                     "res/values/strings.xml"));
     }
+
+    public void testAllowBackup() throws Exception {
+        mEnabled = Collections.singleton(ManifestOrderDetector.ALLOW_BACKUP);
+        assertEquals(
+                "AndroidManifest.xml:9: Warning: Should explicitly set android:allowBackup to " +
+                "true or false (it's true by default, and that can have some security " +
+                "implications for the application's data) [AllowBackup]\n" +
+                "    <application\n" +
+                "    ^\n" +
+                "0 errors, 1 warnings\n",
+                lintProject(
+                        "AndroidManifest.xml",
+                        "res/values/strings.xml"));
+    }
+
+    public void testAllowBackupOk() throws Exception {
+        mEnabled = Collections.singleton(ManifestOrderDetector.ALLOW_BACKUP);
+        assertEquals(
+                "No warnings.",
+                lintProject(
+                        "allowbackup.xml=>AndroidManifest.xml",
+                        "res/values/strings.xml"));
+    }
+
+    public void testAllowIgnore() throws Exception {
+        mEnabled = Collections.singleton(ManifestOrderDetector.ALLOW_BACKUP);
+        assertEquals(
+                "No warnings.",
+                lintProject(
+                        "allowbackup_ignore.xml=>AndroidManifest.xml",
+                        "res/values/strings.xml"));
+    }
+
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/allowbackup.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/allowbackup.xml
new file mode 100644
index 0000000..2a95252
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/allowbackup.xml
@@ -0,0 +1,24 @@
+<?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"
+        android:allowBackup="true" >
+        <activity
+            android:label="@string/app_name"
+            android:name=".Foo2Activity" >
+            <intent-filter >
+                <action android:name="android.intent.action.MAIN" />
+
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+    </application>
+
+</manifest>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/allowbackup_ignore.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/allowbackup_ignore.xml
new file mode 100644
index 0000000..72f8bb4
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/allowbackup_ignore.xml
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    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"
+        tools:ignore="AllowBackup"
+        <activity
+            android:label="@string/app_name"
+            android:name=".Foo2Activity" >
+            <intent-filter >
+                <action android:name="android.intent.action.MAIN" />
+
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+    </application>
+
+</manifest>