Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 93fc5270 authored by Jeff Sharkey's avatar Jeff Sharkey Committed by Android (Google) Code Review
Browse files

Merge changes from topic "jun24" 

* changes:
  Add checker for PID/UID/user ID arguments.
  Add checker to support createUserContext().
  Add tests for framework-specific Error Prone.
parents 703b325c 439b8616

errorprone/Android.bp

+22 −0
Original line number Diff line number Diff line
@@ -21,3 +21,25 @@ java_library_host { 
        "//external/dagger2:dagger2-auto-service",

    ],

}



java_test_host {

    name: "error_prone_android_framework_test",

    test_suites: ["general-tests"],

    srcs: ["tests/java/**/*.java"],

    java_resource_dirs: ["tests/res"],

    java_resources: [":error_prone_android_framework_testdata"],

    static_libs: [

        "error_prone_android_framework_lib",

        "error_prone_test_helpers",

        "hamcrest-library",

        "hamcrest",

        "platform-test-annotations",

        "junit",

    ],

}



filegroup {

    name: "error_prone_android_framework_testdata",

    path: "tests/res",

    srcs: ["tests/res/**/*.java"],

}

errorprone/TEST_MAPPING

0 → 100644
+7 −0
Original line number Diff line number Diff line 
{

    "presubmit": [

        {

            "name": "error_prone_android_framework_test"

        }

    ]

}

errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java

0 → 100644
+100 −0
Original line number Diff line number Diff line 
/*

 * 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.enclosingClass;

import static com.google.errorprone.matchers.Matchers.hasAnnotation;

import static com.google.errorprone.matchers.Matchers.instanceMethod;

import static com.google.errorprone.matchers.Matchers.methodInvocation;



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.google.errorprone.util.ASTHelpers;

import com.sun.source.tree.ExpressionTree;

import com.sun.source.tree.MethodInvocationTree;

import com.sun.source.tree.Tree;

import com.sun.tools.javac.code.Symbol.VarSymbol;



import java.util.List;

import java.util.Locale;

import java.util.function.Predicate;



/**

 * To avoid an explosion of {@code startActivityForUser} style methods, we've

 * converged on recommending the use of {@code Context.createContextAsUser()},

 * and then ensuring that all system services pass {@link Context.getUserId()}

 * for any {@code int userId} arguments across Binder interfaces.

 * <p>

 * This design allows developers to easily redirect all services obtained from a

 * specific {@code Context} to a different user with no additional API surface.

 */

@AutoService(BugChecker.class)

@BugPattern(

    name = "AndroidFrameworkContextUserId",

    summary = "Verifies that system_server calls use Context.getUserId()",

    severity = WARNING)

public final class ContextUserIdChecker extends BugChecker implements MethodInvocationTreeMatcher {

    private static final Matcher<Tree> INSIDE_MANAGER =

            enclosingClass(hasAnnotation("android.annotation.SystemService"));



    private static final Matcher<ExpressionTree> BINDER_CALL = methodInvocation(

            instanceMethod().onDescendantOf("android.os.IInterface").withAnyName());

    private static final Matcher<ExpressionTree> GET_USER_ID_CALL = methodInvocation(

            instanceMethod().onDescendantOf("android.content.Context").named("getUserId"));



    @Override

    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {

        if (INSIDE_MANAGER.matches(tree, state)

                && BINDER_CALL.matches(tree, state)) {

            final List<VarSymbol> vars = ASTHelpers.getSymbol(tree).params();

            for (int i = 0; i < vars.size(); i++) {

                if (USER_ID_VAR.test(vars.get(i)) &&

                        !GET_USER_ID_CALL.matches(tree.getArguments().get(i), state)) {

                    return buildDescription(tree)

                            .setMessage("Must pass Context.getUserId() as user ID"

                                    + "to enable createContextAsUser()")

                            .build();

                }

            }

        }

        return Description.NO_MATCH;

    }



    private static final UserIdMatcher USER_ID_VAR = new UserIdMatcher();



    private static class UserIdMatcher implements Predicate<VarSymbol> {

        @Override

        public boolean test(VarSymbol t) {

            if ("int".equals(t.type.toString())) {

                switch (t.name.toString().toLowerCase(Locale.ROOT)) {

                    case "user":

                    case "userid":

                    case "userhandle":

                    case "user_id":

                        return true;

                }

            }

            return false;

        }

    }

}

errorprone/java/com/google/errorprone/bugpatterns/android/UidChecker.java

0 → 100644
+108 −0
Original line number Diff line number Diff line 
/*

 * 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;

    }

}

errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java

0 → 100644
+82 −0
Original line number Diff line number Diff line 
/*

 * 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 ContextUserIdCheckerTest {

    private CompilationTestHelper compilationHelper;



    @Before

    public void setUp() {

        compilationHelper = CompilationTestHelper.newInstance(

                ContextUserIdChecker.class, getClass());

    }



    @Test

    public void testValid() {

        compilationHelper

                .addSourceFile("/android/annotation/SystemService.java")

                .addSourceFile("/android/content/Context.java")

                .addSourceFile("/android/foo/IFooService.java")

                .addSourceFile("/android/os/IInterface.java")

                .addSourceFile("/android/os/RemoteException.java")

                .addSourceLines("FooManager.java",

                        "import android.annotation.SystemService;",

                        "import android.content.Context;",

                        "import android.foo.IFooService;",

                        "import android.os.RemoteException;",

                        "@SystemService(\"foo\") public class FooManager {",

                        "  Context mContext;",

                        "  IFooService mService;",

                        "  void bar() throws RemoteException {",

                        "    mService.baz(null, mContext.getUserId());",

                        "  }",

                        "}")

                .doTest();

    }



    @Test

    public void testInvalid() {

        compilationHelper

                .addSourceFile("/android/annotation/SystemService.java")

                .addSourceFile("/android/content/Context.java")

                .addSourceFile("/android/foo/IFooService.java")

                .addSourceFile("/android/os/IInterface.java")

                .addSourceFile("/android/os/RemoteException.java")

                .addSourceLines("FooManager.java",

                        "import android.annotation.SystemService;",

                        "import android.content.Context;",

                        "import android.foo.IFooService;",

                        "import android.os.RemoteException;",

                        "@SystemService(\"foo\") public class FooManager {",

                        "  Context mContext;",

                        "  IFooService mService;",

                        "  void bar() throws RemoteException {",

                        "    // BUG: Diagnostic contains:",

                        "    mService.baz(null, 0);",

                        "  }",

                        "}")

                .doTest();

    }

}