Detector for Binder.clearCallingIdentity() bugs.

Binder maintains thread-local identity information about any remote
caller, which can be temporarily cleared while performing operations
that need to be handled as the current process. However, it's
important to restore the original remote calling identity after
carefully scoping this work inside a try/finally block, to avoid
obscure security vulnerabilities.

Bug: 155703208
Test: atest error_prone_android_framework_test
Change-Id: I568771a50af27637e4984950dcada2248ce16afe

3 files changed, 227 insertions, 0 deletions

diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/BinderIdentityChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/BinderIdentityChecker.java
new file mode 100644
index 000000000000..68477edf97d1
--- /dev/null
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/BinderIdentityChecker.java
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns.android;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Matchers.contains;
+import static com.google.errorprone.matchers.Matchers.methodInvocation;
+import static com.google.errorprone.matchers.Matchers.staticMethod;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.BlockTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.StatementTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
+import com.sun.source.tree.TryTree;
+import com.sun.source.tree.VariableTree;
+
+import java.util.List;
+
+import javax.lang.model.element.Modifier;
+
+/**
+ * Binder maintains thread-local identity information about any remote caller,
+ * which can be temporarily cleared while performing operations that need to be
+ * handled as the current process. However, it's important to restore the
+ * original remote calling identity after carefully scoping this work inside a
+ * try/finally block, to avoid obscure security vulnerabilities.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "AndroidFrameworkBinderIdentity",
+    summary = "Verifies that Binder.clearCallingIdentity() is always restored",
+    severity = WARNING)
+public final class BinderIdentityChecker extends BugChecker implements MethodInvocationTreeMatcher {
+    private static final Matcher<ExpressionTree> CLEAR_CALL = methodInvocation(staticMethod()
+            .onClass("android.os.Binder").withSignature("clearCallingIdentity()"));
+    private static final Matcher<ExpressionTree> RESTORE_CALL = methodInvocation(staticMethod()
+            .onClass("android.os.Binder").withSignature("restoreCallingIdentity(long)"));
+
+    @Override
+    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+        if (CLEAR_CALL.matches(tree, state)) {
+            // First, make sure we're recording the token for later
+            final VariableTree token = state.findEnclosing(VariableTree.class);
+            if (token == null || !token.getModifiers().getFlags().contains(Modifier.FINAL)) {
+                return buildDescription(tree)
+                        .setMessage("Must store Binder.clearCallingIdentity() token as final"
+                                + " variable to support safe restore")
+                        .build();
+            }
+
+            // Next, verify the very next block is try-finally; any other calls
+            // between the clearing and try risk throwing an exception without
+            // doing a safe restore
+            final Tree next = nextStatement(token, state);
+            if (next == null || next.getKind() != Kind.TRY) {
+                return buildDescription(tree)
+                        .setMessage("Must immediately define a try-finally block after"
+                                + " Binder.clearCallingIdentity() to support safe restore")
+                        .build();
+            }
+
+            // Finally, verify that we restore inside the finally block
+            final TryTree tryTree = (TryTree) next;
+            final BlockTree finallyTree = tryTree.getFinallyBlock();
+            if (finallyTree == null
+                    || !contains(ExpressionTree.class, RESTORE_CALL).matches(finallyTree, state)) {
+                return buildDescription(tree)
+                        .setMessage("Must call Binder.restoreCallingIdentity() in finally"
+                                + "  block to support safe restore")
+                        .build();
+            }
+        }
+        return Description.NO_MATCH;
+    }
+
+    private static Tree nextStatement(Tree tree, VisitorState state) {
+        final BlockTree block = state.findEnclosing(BlockTree.class);
+        if (block == null) return null;
+        final List<? extends StatementTree> siblings = block.getStatements();
+        if (siblings == null) return null;
+        final int index = siblings.indexOf(tree);
+        if (index == -1 || index + 1 >= siblings.size()) return null;
+        return siblings.get(index + 1);
+    }
+}
diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/BinderIdentityCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/BinderIdentityCheckerTest.java
new file mode 100644
index 000000000000..9448344f7abb
--- /dev/null
+++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/BinderIdentityCheckerTest.java
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns.android;
+
+import com.google.errorprone.CompilationTestHelper;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class BinderIdentityCheckerTest {
+    private CompilationTestHelper compilationHelper;
+
+    @Before
+    public void setUp() {
+        compilationHelper = CompilationTestHelper.newInstance(
+                BinderIdentityChecker.class, getClass());
+    }
+
+    @Test
+    public void testValid() {
+        compilationHelper
+                .addSourceFile("/android/os/Binder.java")
+                .addSourceLines("FooService.java",
+                        "import android.os.Binder;",
+                        "public class FooService {",
+                        "  void bar() {",
+                        "    final long token = Binder.clearCallingIdentity();",
+                        "    try {",
+                        "      FooService.class.toString();",
+                        "    } finally {",
+                        "      Binder.restoreCallingIdentity(token);",
+                        "    }",
+                        "  }",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testInvalid() {
+        compilationHelper
+                .addSourceFile("/android/os/Binder.java")
+                .addSourceLines("FooService.java",
+                        "import android.os.Binder;",
+                        "public class FooService {",
+                        "  void noRestore() {",
+                        "    // BUG: Diagnostic contains:",
+                        "    final long token = Binder.clearCallingIdentity();",
+                        "    FooService.class.toString();",
+                        "  }",
+                        "  void noTry() {",
+                        "    // BUG: Diagnostic contains:",
+                        "    final long token = Binder.clearCallingIdentity();",
+                        "    FooService.class.toString();",
+                        "    Binder.restoreCallingIdentity(token);",
+                        "  }",
+                        "  void noImmediateTry() {",
+                        "    // BUG: Diagnostic contains:",
+                        "    final long token = Binder.clearCallingIdentity();",
+                        "    FooService.class.toString();",
+                        "    try {",
+                        "      FooService.class.toString();",
+                        "    } finally {",
+                        "      Binder.restoreCallingIdentity(token);",
+                        "    }",
+                        "  }",
+                        "  void noFinally() {",
+                        "    // BUG: Diagnostic contains:",
+                        "    final long token = Binder.clearCallingIdentity();",
+                        "    try {",
+                        "      FooService.class.toString();",
+                        "    } catch (Exception ignored) { }",
+                        "  }",
+                        "  void noFinal() {",
+                        "    // BUG: Diagnostic contains:",
+                        "    long token = Binder.clearCallingIdentity();",
+                        "    try {",
+                        "      FooService.class.toString();",
+                        "    } finally {",
+                        "      Binder.restoreCallingIdentity(token);",
+                        "    }",
+                        "  }",
+                        "  void noRecording() {",
+                        "    // BUG: Diagnostic contains:",
+                        "    Binder.clearCallingIdentity();",
+                        "    FooService.class.toString();",
+                        "  }",
+                        "  void noWork() {",
+                        "    // BUG: Diagnostic contains:",
+                        "    final long token = Binder.clearCallingIdentity();",
+                        "  }",
+                        "}")
+                .doTest();
+    }
+}
diff --git a/errorprone/tests/res/android/os/Binder.java b/errorprone/tests/res/android/os/Binder.java
index d388587c2f58..c969108bd425 100644
--- a/errorprone/tests/res/android/os/Binder.java
+++ b/errorprone/tests/res/android/os/Binder.java
@@ -20,4 +20,12 @@ public class Binder {
     public static int getCallingUid() {
         throw new UnsupportedOperationException();
     }
+
+    public static long clearCallingIdentity() {
+        throw new UnsupportedOperationException();
+    }
+
+    public static void restoreCallingIdentity(long token) {
+        throw new UnsupportedOperationException();
+    }
 }