Add checker for PID/UID/user ID arguments.
Many system internals pass around PID, UID and user ID arguments as a single weakly-typed "int" value, which developers can accidentally cross in method argument lists, resulting in obscure bugs. Bug: 155703208 Test: atest error_prone_android_framework_test Change-Id: I5e4d9b5a533071f94d82dff17faff5d52ae54564
This commit is contained in:
Jeff Sharkey
@@ -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 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.util.ASTHelpers;
|
||||
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.tools.javac.code.Symbol.VarSymbol;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
/**
|
||||
* Many system internals pass around PID, UID and user ID arguments as a single
|
||||
* weakly-typed {@code int} value, which developers can accidentally cross in
|
||||
* method argument lists, resulting in obscure bugs.
|
||||
*/
|
||||
@AutoService(BugChecker.class)
|
||||
@BugPattern(
|
||||
name = "AndroidFrameworkUid",
|
||||
summary = "Verifies that PID, UID and user ID arguments aren't crossed",
|
||||
severity = WARNING)
|
||||
public final class UidChecker extends BugChecker implements MethodInvocationTreeMatcher {
|
||||
@Override
|
||||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
|
||||
final List<VarSymbol> vars = ASTHelpers.getSymbol(tree).params();
|
||||
final List<? extends ExpressionTree> args = tree.getArguments();
|
||||
for (int i = 0; i < Math.min(vars.size(), args.size()); i++) {
|
||||
final Flavor varFlavor = getFlavor(vars.get(i));
|
||||
final Flavor argFlavor = getFlavor(args.get(i));
|
||||
if (varFlavor == Flavor.UNKNOWN || argFlavor == Flavor.UNKNOWN) {
|
||||
continue;
|
||||
}
|
||||
if (varFlavor != argFlavor) {
|
||||
return buildDescription(tree).setMessage("Argument #" + (i + 1) + " expected "
|
||||
+ varFlavor + " but passed " + argFlavor).build();
|
||||
}
|
||||
}
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
private static enum Flavor {
|
||||
UNKNOWN(null),
|
||||
PID(Pattern.compile("(^pid$|Pid$)")),
|
||||
UID(Pattern.compile("(^uid$|Uid$)")),
|
||||
USER_ID(Pattern.compile("(^userId$|UserId$|^userHandle$|UserHandle$)"));
|
||||
|
||||
private Pattern pattern;
|
||||
private Flavor(Pattern pattern) {
|
||||
this.pattern = pattern;
|
||||
}
|
||||
public boolean matches(CharSequence input) {
|
||||
return (pattern != null) && pattern.matcher(input).find();
|
||||
}
|
||||
}
|
||||
|
||||
private static Flavor getFlavor(String name) {
|
||||
for (Flavor f : Flavor.values()) {
|
||||
if (f.matches(name)) {
|
||||
return f;
|
||||
}
|
||||
}
|
||||
return Flavor.UNKNOWN;
|
||||
}
|
||||
|
||||
private static Flavor getFlavor(VarSymbol symbol) {
|
||||
final String type = symbol.type.toString();
|
||||
if ("int".equals(type)) {
|
||||
return getFlavor(symbol.name.toString());
|
||||
}
|
||||
return Flavor.UNKNOWN;
|
||||
}
|
||||
|
||||
private static Flavor getFlavor(ExpressionTree tree) {
|
||||
if (tree instanceof IdentifierTree) {
|
||||
return getFlavor(((IdentifierTree) tree).getName().toString());
|
||||
} else if (tree instanceof MemberSelectTree) {
|
||||
return getFlavor(((MemberSelectTree) tree).getIdentifier().toString());
|
||||
} else if (tree instanceof MethodInvocationTree) {
|
||||
return getFlavor(((MethodInvocationTree) tree).getMethodSelect());
|
||||
}
|
||||
return Flavor.UNKNOWN;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,101 @@
|
||||
/*
|
||||
* 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 UidCheckerTest {
|
||||
private CompilationTestHelper compilationHelper;
|
||||
|
||||
@Before
|
||||
public void setUp() {
|
||||
compilationHelper = CompilationTestHelper.newInstance(
|
||||
UidChecker.class, getClass());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTypical() {
|
||||
compilationHelper
|
||||
.addSourceLines("Example.java",
|
||||
"public abstract class Example {",
|
||||
" abstract void bar(int pid, int uid, int userId);",
|
||||
" abstract int getUserId();",
|
||||
" void foo(int pid, int uid, int userId, int unrelated) {",
|
||||
" bar(0, 0, 0);",
|
||||
" bar(pid, uid, userId);",
|
||||
" bar(pid, uid, getUserId());",
|
||||
" bar(unrelated, unrelated, unrelated);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" bar(uid, pid, userId);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" bar(pid, userId, uid);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" bar(getUserId(), 0, 0);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCallingUid() {
|
||||
compilationHelper
|
||||
.addSourceFile("/android/os/Binder.java")
|
||||
.addSourceFile("/android/os/UserHandle.java")
|
||||
.addSourceLines("Example.java",
|
||||
"import android.os.Binder;",
|
||||
"import android.os.UserHandle;",
|
||||
"public abstract class Example {",
|
||||
" int callingUserId;",
|
||||
" int callingUid;",
|
||||
" abstract void setCallingUserId(int callingUserId);",
|
||||
" abstract void setCallingUid(int callingUid);",
|
||||
" void doUserId(int callingUserId) {",
|
||||
" setCallingUserId(UserHandle.getUserId(Binder.getCallingUid()));",
|
||||
" setCallingUserId(this.callingUserId);",
|
||||
" setCallingUserId(callingUserId);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" setCallingUserId(Binder.getCallingUid());",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" setCallingUserId(this.callingUid);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" setCallingUserId(callingUid);",
|
||||
" }",
|
||||
" void doUid(int callingUserId) {",
|
||||
" setCallingUid(Binder.getCallingUid());",
|
||||
" setCallingUid(this.callingUid);",
|
||||
" setCallingUid(callingUid);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" setCallingUid(UserHandle.getUserId(Binder.getCallingUid()));",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" setCallingUid(this.callingUserId);",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" setCallingUid(callingUserId);",
|
||||
" }",
|
||||
" void doInner() {",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" setCallingUserId(UserHandle.getUserId(callingUserId));",
|
||||
" }",
|
||||
"}")
|
||||
.doTest();
|
||||
}
|
||||
}
|
||||
23
errorprone/tests/res/android/os/Binder.java
Normal file
23
errorprone/tests/res/android/os/Binder.java
Normal file
@@ -0,0 +1,23 @@
|
||||
/*
|
||||
* 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 android.os;
|
||||
|
||||
public class Binder {
|
||||
public static int getCallingUid() {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
}
|
||||
31
errorprone/tests/res/android/os/UserHandle.java
Normal file
31
errorprone/tests/res/android/os/UserHandle.java
Normal file
@@ -0,0 +1,31 @@
|
||||
/*
|
||||
* 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 android.os;
|
||||
|
||||
public class UserHandle {
|
||||
public static int getUserId(int uid) {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
|
||||
public static int getAppId(int uid) {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
|
||||
public static int getUid(int userId, int appId) {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user