diff options
Diffstat (limited to 'errorprone/java')
| -rw-r--r-- | errorprone/java/android/annotation/SdkConstant.java | 36 | ||||
| -rw-r--r-- | errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java | 224 |
2 files changed, 224 insertions, 36 deletions
diff --git a/errorprone/java/android/annotation/SdkConstant.java b/errorprone/java/android/annotation/SdkConstant.java new file mode 100644 index 000000000000..0a5318609847 --- /dev/null +++ b/errorprone/java/android/annotation/SdkConstant.java @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2008 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 android.annotation; + +import java.lang.annotation.Target; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Indicates a constant field value should be exported to be used in the SDK tools. + * @hide + */ +@Target({ ElementType.FIELD }) +@Retention(RetentionPolicy.SOURCE) +public @interface SdkConstant { + public static enum SdkConstantType { + ACTIVITY_INTENT_ACTION, BROADCAST_INTENT_ACTION, SERVICE_ACTION, INTENT_CATEGORY, FEATURE; + } + + SdkConstantType value(); +} diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java index f54782d4eb77..d1e4309c365e 100644 --- a/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java +++ b/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java @@ -26,40 +26,49 @@ import static com.google.errorprone.matchers.Matchers.methodInvocation; import static com.google.errorprone.matchers.Matchers.methodIsNamed; import static com.google.errorprone.matchers.Matchers.staticMethod; +import android.annotation.RequiresPermission; import android.annotation.SuppressLint; import com.google.auto.service.AutoService; +import com.google.common.base.Objects; 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.bugpatterns.BugChecker.MethodTreeMatcher; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; import com.sun.source.util.TreeScanner; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Symbol.ClassSymbol; import com.sun.tools.javac.code.Symbol.MethodSymbol; +import com.sun.tools.javac.code.Symbol.VarSymbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Type.ClassType; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; +import java.util.Optional; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Predicate; import java.util.regex.Pattern; -import javax.lang.model.element.AnnotationMirror; -import javax.lang.model.element.AnnotationValue; -import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Name; /** * Inspects both the client and server side of AIDL interfaces to ensure that @@ -71,9 +80,8 @@ import javax.lang.model.element.ExecutableElement; name = "AndroidFrameworkRequiresPermission", summary = "Verifies that @RequiresPermission annotations are consistent across AIDL", severity = WARNING) -public final class RequiresPermissionChecker extends BugChecker implements MethodTreeMatcher { - private static final String ANNOTATION_REQUIRES_PERMISSION = "RequiresPermission"; - +public final class RequiresPermissionChecker extends BugChecker + implements MethodTreeMatcher, MethodInvocationTreeMatcher { private static final Matcher<ExpressionTree> ENFORCE_VIA_CONTEXT = methodInvocation( instanceMethod() .onDescendantOf("android.content.Context") @@ -110,6 +118,18 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho private static final Matcher<ExpressionTree> RESTORE_CALL = methodInvocation(staticMethod() .onClass("android.os.Binder").withSignature("restoreCallingIdentity(long)")); + private static final Matcher<ExpressionTree> SEND_BROADCAST = methodInvocation( + instanceMethod() + .onDescendantOf("android.content.Context") + .withNameMatching(Pattern.compile("^send(Ordered|Sticky)?Broadcast.*$"))); + private static final Matcher<ExpressionTree> SEND_PENDING_INTENT = methodInvocation( + instanceMethod() + .onDescendantOf("android.app.PendingIntent") + .named("send")); + + private static final Matcher<ExpressionTree> INTENT_SET_ACTION = methodInvocation( + instanceMethod().onDescendantOf("android.content.Intent").named("setAction")); + @Override public Description matchMethod(MethodTree tree, VisitorState state) { // Ignore methods without an implementation @@ -166,7 +186,7 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho // to require; yell if we're too broad if (!actualPerm.containsAll(expectedPerm)) { return buildDescription(tree) - .setMessage("Method " + method.name.toString() + "() annotated " + expectedPerm + .setMessage("Method " + method.name.toString() + "() annotated " + expectedPerm + " but too wide; only invokes methods requiring " + actualPerm) .build(); } @@ -174,6 +194,27 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho return Description.NO_MATCH; } + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (SEND_BROADCAST.matches(tree, state)) { + final ParsedRequiresPermission sourcePerm = + parseBroadcastSourceRequiresPermission(tree, state); + final ParsedRequiresPermission targetPerm = + parseBroadcastTargetRequiresPermission(tree, state); + if (sourcePerm == null) { + return buildDescription(tree) + .setMessage("Failed to resolve broadcast intent action for validation") + .build(); + } else if (!Objects.equal(sourcePerm, targetPerm)) { + return buildDescription(tree) + .setMessage("Broadcast annotated " + sourcePerm + " but protected with " + + targetPerm) + .build(); + } + } + return Description.NO_MATCH; + } + static class ParsedRequiresPermission { final Set<String> allOf = new HashSet<>(); final Set<String> anyOf = new HashSet<>(); @@ -203,6 +244,16 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho } @Override + public boolean equals(Object obj) { + if (obj instanceof ParsedRequiresPermission) { + final ParsedRequiresPermission other = (ParsedRequiresPermission) obj; + return allOf.equals(other.allOf) && anyOf.equals(other.anyOf); + } else { + return false; + } + } + + @Override public String toString() { if (isEmpty()) { return "[none]"; @@ -215,37 +266,143 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho return res; } + public static ParsedRequiresPermission from(RequiresPermission perm) { + final ParsedRequiresPermission res = new ParsedRequiresPermission(); + res.addAll(perm); + return res; + } + public void addAll(ParsedRequiresPermission perm) { + if (perm == null) return; this.allOf.addAll(perm.allOf); this.anyOf.addAll(perm.anyOf); } - public void addAll(AnnotationMirror a) { - for (Map.Entry<? extends ExecutableElement, ? extends AnnotationValue> entry : a - .getElementValues().entrySet()) { - if (entry.getKey().getSimpleName().contentEquals("value")) { - maybeAdd(allOf, entry.getValue()); - } else if (entry.getKey().getSimpleName().contentEquals("allOf")) { - maybeAdd(allOf, entry.getValue()); - } else if (entry.getKey().getSimpleName().contentEquals("anyOf")) { - maybeAdd(anyOf, entry.getValue()); - } + public void addAll(RequiresPermission perm) { + if (perm == null) return; + if (!perm.value().isEmpty()) this.allOf.add(perm.value()); + if (perm.allOf() != null) this.allOf.addAll(Arrays.asList(perm.allOf())); + if (perm.anyOf() != null) this.anyOf.addAll(Arrays.asList(perm.anyOf())); + } + + public void addConstValue(Tree tree) { + final Object value = ASTHelpers.constValue(tree); + if (value != null) { + allOf.add(String.valueOf(value)); } } + } - private static void maybeAdd(Set<String> set, Object value) { - if (value instanceof AnnotationValue) { - maybeAdd(set, ((AnnotationValue) value).getValue()); - } else if (value instanceof String) { - set.add((String) value); - } else if (value instanceof Collection) { - for (Object o : (Collection) value) { - maybeAdd(set, o); - } - } else { - throw new RuntimeException(String.valueOf(value.getClass())); + private static ExpressionTree findArgumentByParameterName(MethodInvocationTree tree, + Predicate<String> paramName) { + final MethodSymbol sym = ASTHelpers.getSymbol(tree); + final List<VarSymbol> params = sym.getParameters(); + for (int i = 0; i < params.size(); i++) { + if (paramName.test(params.get(i).name.toString())) { + return tree.getArguments().get(i); } } + return null; + } + + private static Name resolveName(ExpressionTree tree) { + if (tree instanceof IdentifierTree) { + return ((IdentifierTree) tree).getName(); + } else if (tree instanceof MemberSelectTree) { + return resolveName(((MemberSelectTree) tree).getExpression()); + } else { + return null; + } + } + + private static ParsedRequiresPermission parseIntentAction(NewClassTree tree) { + final Optional<? extends ExpressionTree> arg = tree.getArguments().stream().findFirst(); + if (arg.isPresent()) { + return ParsedRequiresPermission.from( + ASTHelpers.getAnnotation(arg.get(), RequiresPermission.class)); + } else { + return null; + } + } + + private static ParsedRequiresPermission parseIntentAction(MethodInvocationTree tree) { + return ParsedRequiresPermission.from(ASTHelpers.getAnnotation( + tree.getArguments().get(0), RequiresPermission.class)); + } + + private static ParsedRequiresPermission parseBroadcastSourceRequiresPermission( + MethodInvocationTree methodTree, VisitorState state) { + final ExpressionTree arg = findArgumentByParameterName(methodTree, + (name) -> name.toLowerCase().contains("intent")); + if (arg instanceof IdentifierTree) { + final Name argName = ((IdentifierTree) arg).getName(); + final MethodTree method = state.findEnclosing(MethodTree.class); + final AtomicReference<ParsedRequiresPermission> res = new AtomicReference<>(); + method.accept(new TreeScanner<Void, Void>() { + private ParsedRequiresPermission last; + + @Override + public Void visitMethodInvocation(MethodInvocationTree tree, Void param) { + if (Objects.equal(methodTree, tree)) { + res.set(last); + } else { + final Name name = resolveName(tree.getMethodSelect()); + if (Objects.equal(argName, name) + && INTENT_SET_ACTION.matches(tree, state)) { + last = parseIntentAction(tree); + } + } + return super.visitMethodInvocation(tree, param); + } + + @Override + public Void visitAssignment(AssignmentTree tree, Void param) { + final Name name = resolveName(tree.getVariable()); + final Tree init = tree.getExpression(); + if (Objects.equal(argName, name) + && init instanceof NewClassTree) { + last = parseIntentAction((NewClassTree) init); + } + return super.visitAssignment(tree, param); + } + + @Override + public Void visitVariable(VariableTree tree, Void param) { + final Name name = tree.getName(); + final ExpressionTree init = tree.getInitializer(); + if (Objects.equal(argName, name) + && init instanceof NewClassTree) { + last = parseIntentAction((NewClassTree) init); + } + return super.visitVariable(tree, param); + } + }, null); + return res.get(); + } + return null; + } + + private static ParsedRequiresPermission parseBroadcastTargetRequiresPermission( + MethodInvocationTree tree, VisitorState state) { + final ExpressionTree arg = findArgumentByParameterName(tree, + (name) -> name.toLowerCase().contains("permission")); + final ParsedRequiresPermission res = new ParsedRequiresPermission(); + if (arg != null) { + arg.accept(new TreeScanner<Void, Void>() { + @Override + public Void visitIdentifier(IdentifierTree tree, Void param) { + res.addConstValue(tree); + return super.visitIdentifier(tree, param); + } + + @Override + public Void visitMemberSelect(MemberSelectTree tree, Void param) { + res.addConstValue(tree); + return super.visitMemberSelect(tree, param); + } + }, null); + } + return res; } private static ParsedRequiresPermission parseRequiresPermissionRecursively( @@ -276,12 +433,7 @@ public final class RequiresPermissionChecker extends BugChecker implements Metho final ParsedRequiresPermission res = new ParsedRequiresPermission(); for (MethodSymbol symbol : symbols) { - for (AnnotationMirror a : symbol.getAnnotationMirrors()) { - if (a.getAnnotationType().asElement().getSimpleName() - .contentEquals(ANNOTATION_REQUIRES_PERMISSION)) { - res.addAll(a); - } - } + res.addAll(symbol.getAnnotation(RequiresPermission.class)); } return res; } |