diff --git a/tools/lint/global/Android.bp b/tools/lint/global/Android.bp index 3756abea23301..4a42c09ab4e58 100644 --- a/tools/lint/global/Android.bp +++ b/tools/lint/global/Android.bp @@ -43,7 +43,7 @@ java_test_host { // compiled for java 15. The soong build doesn't support // java 15 yet, so we can't compile against "lint". Disable // the test until java 15 is supported. - enabled: false, + // enabled: false, srcs: ["checks/src/test/java/**/*.kt"], static_libs: [ "AndroidGlobalLintChecker", diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt index bba819cd9096e..bcef9171aa3c3 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt @@ -114,6 +114,13 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { val overridingName = "${overridingClass.name}.${overridingMethod.name}" val overriddenName = "${overriddenClass.name}.${overriddenMethod.name}" if (overridingAnnotation == null) { + if (shouldIgnoreGeneratedMethod( + context, + overriddenClass = overriddenClass, + overridingClass = overridingClass) + ) { + return + } val msg = "The method $overridingName overrides the method $overriddenName which " + "is annotated with @EnforcePermission. The same annotation must be used " + "on $overridingName" @@ -215,6 +222,39 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner { } } + /** + * since this lint runs globally, it will also run against generated + * test code e.g. + * system/tools/aidl/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtected.java + * system/tools/aidl/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtectedInterface.java + * we do not want to report errors against generated `Stub` and `Proxy` classes in those files + */ + private fun shouldIgnoreGeneratedMethod( + context: JavaContext, + overriddenClass: PsiClass, + overridingClass: PsiClass, + + ): Boolean { + if (isInterfaceAndExtendsIInterface(overriddenClass) && + context.evaluator.isStatic(overridingClass)) { + if (overridingClass.name == "Default") return true + if (overridingClass.name == "Proxy") { + val shouldBeStub = overridingClass.parent as? PsiClass ?: return false + return shouldBeStub.name == "Stub" && + context.evaluator.isAbstract(shouldBeStub) && + context.evaluator.isStatic(shouldBeStub) && + shouldBeStub.extendsList?.referenceElements + ?.any { it.qualifiedName == BINDER_CLASS } == true + } + } + return false + } + + private fun isInterfaceAndExtendsIInterface(overriddenClass: PsiClass): Boolean = + overriddenClass.isInterface && + overriddenClass.extendsList?.referenceElements + ?.any { it.qualifiedName == IINTERFACE_INTERFACE } == true + companion object { val EXPLANATION = """ The @EnforcePermission annotation is used to indicate that the underlying binder code diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt index 3c2ea1db0ad6f..c1e47e906e224 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt @@ -28,8 +28,8 @@ import com.android.tools.lint.detector.api.SourceCodeScanner import com.intellij.psi.PsiElement import org.jetbrains.uast.UBlockExpression import org.jetbrains.uast.UDeclarationsExpression -import org.jetbrains.uast.UExpression import org.jetbrains.uast.UElement +import org.jetbrains.uast.UExpression import org.jetbrains.uast.UMethod class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner { @@ -40,6 +40,7 @@ class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner { private inner class AidlStubHandler(val context: JavaContext) : UElementHandler() { override fun visitMethod(node: UMethod) { + if (context.evaluator.isAbstract(node)) return if (!node.hasAnnotation(ANNOTATION_ENFORCE_PERMISSION)) return val targetExpression = "super.${node.name}$HELPER_SUFFIX()" diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorCodegenTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorCodegenTest.kt new file mode 100644 index 0000000000000..f2930d9faac75 --- /dev/null +++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorCodegenTest.kt @@ -0,0 +1,557 @@ +/* + * Copyright (C) 2022 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.android.lint.aidl + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestFile +import com.android.tools.lint.checks.infrastructure.TestLintTask +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue + +@Suppress("UnstableApiUsage") +class EnforcePermissionDetectorCodegenTest : LintDetectorTest() { + override fun getDetector(): Detector = EnforcePermissionDetector() + + override fun getIssues(): List = listOf( + EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION, + EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION + ) + + override fun lint(): TestLintTask = super.lint().allowMissingSdk(true) + + fun test_generated_IProtected() { + lint().files( + java( + """ + /* + * This file is auto-generated. DO NOT MODIFY. + */ + package android.aidl.tests.permission; + public interface IProtected extends android.os.IInterface + { + /** Default implementation for IProtected. */ + public static class Default implements android.aidl.tests.permission.IProtected + { + @Override public void PermissionProtected() throws android.os.RemoteException + { + } + @Override public void MultiplePermissionsAll() throws android.os.RemoteException + { + } + @Override public void MultiplePermissionsAny() throws android.os.RemoteException + { + } + @Override public void NonManifestPermission() throws android.os.RemoteException + { + } + // Used by the integration tests to dynamically set permissions that are considered granted. + @Override public void SetGranted(java.util.List permissions) throws android.os.RemoteException + { + } + @Override + public android.os.IBinder asBinder() { + return null; + } + } + /** Local-side IPC implementation stub class. */ + public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtected + { + private final android.os.PermissionEnforcer mEnforcer; + /** Construct the stub using the Enforcer provided. */ + public Stub(android.os.PermissionEnforcer enforcer) + { + this.attachInterface(this, DESCRIPTOR); + if (enforcer == null) { + throw new IllegalArgumentException("enforcer cannot be null"); + } + mEnforcer = enforcer; + } + @Deprecated + /** Default constructor. */ + public Stub() { + this(android.os.PermissionEnforcer.fromContext( + android.app.ActivityThread.currentActivityThread().getSystemContext())); + } + /** + * Cast an IBinder object into an android.aidl.tests.permission.IProtected interface, + * generating a proxy if needed. + */ + public static android.aidl.tests.permission.IProtected asInterface(android.os.IBinder obj) + { + if ((obj==null)) { + return null; + } + android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR); + if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtected))) { + return ((android.aidl.tests.permission.IProtected)iin); + } + return new android.aidl.tests.permission.IProtected.Stub.Proxy(obj); + } + @Override public android.os.IBinder asBinder() + { + return this; + } + /** @hide */ + public static java.lang.String getDefaultTransactionName(int transactionCode) + { + switch (transactionCode) + { + case TRANSACTION_PermissionProtected: + { + return "PermissionProtected"; + } + case TRANSACTION_MultiplePermissionsAll: + { + return "MultiplePermissionsAll"; + } + case TRANSACTION_MultiplePermissionsAny: + { + return "MultiplePermissionsAny"; + } + case TRANSACTION_NonManifestPermission: + { + return "NonManifestPermission"; + } + case TRANSACTION_SetGranted: + { + return "SetGranted"; + } + default: + { + return null; + } + } + } + /** @hide */ + public java.lang.String getTransactionName(int transactionCode) + { + return this.getDefaultTransactionName(transactionCode); + } + @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException + { + java.lang.String descriptor = DESCRIPTOR; + if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) { + data.enforceInterface(descriptor); + } + switch (code) + { + case INTERFACE_TRANSACTION: + { + reply.writeString(descriptor); + return true; + } + } + switch (code) + { + case TRANSACTION_PermissionProtected: + { + this.PermissionProtected(); + reply.writeNoException(); + break; + } + case TRANSACTION_MultiplePermissionsAll: + { + this.MultiplePermissionsAll(); + reply.writeNoException(); + break; + } + case TRANSACTION_MultiplePermissionsAny: + { + this.MultiplePermissionsAny(); + reply.writeNoException(); + break; + } + case TRANSACTION_NonManifestPermission: + { + this.NonManifestPermission(); + reply.writeNoException(); + break; + } + case TRANSACTION_SetGranted: + { + java.util.List _arg0; + _arg0 = data.createStringArrayList(); + data.enforceNoDataAvail(); + this.SetGranted(_arg0); + reply.writeNoException(); + break; + } + default: + { + return super.onTransact(code, data, reply, flags); + } + } + return true; + } + private static class Proxy implements android.aidl.tests.permission.IProtected + { + private android.os.IBinder mRemote; + Proxy(android.os.IBinder remote) + { + mRemote = remote; + } + @Override public android.os.IBinder asBinder() + { + return mRemote; + } + public java.lang.String getInterfaceDescriptor() + { + return DESCRIPTOR; + } + @Override public void PermissionProtected() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_PermissionProtected, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void MultiplePermissionsAll() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAll, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void MultiplePermissionsAny() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAny, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void NonManifestPermission() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_NonManifestPermission, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + // Used by the integration tests to dynamically set permissions that are considered granted. + @Override public void SetGranted(java.util.List permissions) throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + _data.writeStringList(permissions); + boolean _status = mRemote.transact(Stub.TRANSACTION_SetGranted, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + } + static final int TRANSACTION_PermissionProtected = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0); + /** Helper method to enforce permissions for PermissionProtected */ + protected void PermissionProtected_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, source); + } + static final int TRANSACTION_MultiplePermissionsAll = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1); + /** Helper method to enforce permissions for MultiplePermissionsAll */ + protected void MultiplePermissionsAll_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source); + } + static final int TRANSACTION_MultiplePermissionsAny = (android.os.IBinder.FIRST_CALL_TRANSACTION + 2); + /** Helper method to enforce permissions for MultiplePermissionsAny */ + protected void MultiplePermissionsAny_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermissionAnyOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source); + } + static final int TRANSACTION_NonManifestPermission = (android.os.IBinder.FIRST_CALL_TRANSACTION + 3); + /** Helper method to enforce permissions for NonManifestPermission */ + protected void NonManifestPermission_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, source); + } + static final int TRANSACTION_SetGranted = (android.os.IBinder.FIRST_CALL_TRANSACTION + 4); + /** @hide */ + public int getMaxTransactionId() + { + return 4; + } + } + + @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + public void PermissionProtected() throws android.os.RemoteException; + @android.annotation.EnforcePermission(allOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}) + public void MultiplePermissionsAll() throws android.os.RemoteException; + @android.annotation.EnforcePermission(anyOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}) + public void MultiplePermissionsAny() throws android.os.RemoteException; + @android.annotation.EnforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) + public void NonManifestPermission() throws android.os.RemoteException; + // Used by the integration tests to dynamically set permissions that are considered granted. + @android.annotation.RequiresNoPermission + public void SetGranted(java.util.List permissions) throws android.os.RemoteException; + } + """ + ).indented(), + *stubs + ) + .run() + .expectClean() + } + + fun test_generated_IProtectedInterface() { + lint().files( + java( + """ + /* + * This file is auto-generated. DO NOT MODIFY. + */ + package android.aidl.tests.permission; + public interface IProtectedInterface extends android.os.IInterface + { + /** Default implementation for IProtectedInterface. */ + public static class Default implements android.aidl.tests.permission.IProtectedInterface + { + @Override public void Method1() throws android.os.RemoteException + { + } + @Override public void Method2() throws android.os.RemoteException + { + } + @Override + public android.os.IBinder asBinder() { + return null; + } + } + /** Local-side IPC implementation stub class. */ + public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtectedInterface + { + private final android.os.PermissionEnforcer mEnforcer; + /** Construct the stub using the Enforcer provided. */ + public Stub(android.os.PermissionEnforcer enforcer) + { + this.attachInterface(this, DESCRIPTOR); + if (enforcer == null) { + throw new IllegalArgumentException("enforcer cannot be null"); + } + mEnforcer = enforcer; + } + @Deprecated + /** Default constructor. */ + public Stub() { + this(android.os.PermissionEnforcer.fromContext( + android.app.ActivityThread.currentActivityThread().getSystemContext())); + } + /** + * Cast an IBinder object into an android.aidl.tests.permission.IProtectedInterface interface, + * generating a proxy if needed. + */ + public static android.aidl.tests.permission.IProtectedInterface asInterface(android.os.IBinder obj) + { + if ((obj==null)) { + return null; + } + android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR); + if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtectedInterface))) { + return ((android.aidl.tests.permission.IProtectedInterface)iin); + } + return new android.aidl.tests.permission.IProtectedInterface.Stub.Proxy(obj); + } + @Override public android.os.IBinder asBinder() + { + return this; + } + /** @hide */ + public static java.lang.String getDefaultTransactionName(int transactionCode) + { + switch (transactionCode) + { + case TRANSACTION_Method1: + { + return "Method1"; + } + case TRANSACTION_Method2: + { + return "Method2"; + } + default: + { + return null; + } + } + } + /** @hide */ + public java.lang.String getTransactionName(int transactionCode) + { + return this.getDefaultTransactionName(transactionCode); + } + @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException + { + java.lang.String descriptor = DESCRIPTOR; + if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) { + data.enforceInterface(descriptor); + } + switch (code) + { + case INTERFACE_TRANSACTION: + { + reply.writeString(descriptor); + return true; + } + } + switch (code) + { + case TRANSACTION_Method1: + { + this.Method1(); + reply.writeNoException(); + break; + } + case TRANSACTION_Method2: + { + this.Method2(); + reply.writeNoException(); + break; + } + default: + { + return super.onTransact(code, data, reply, flags); + } + } + return true; + } + private static class Proxy implements android.aidl.tests.permission.IProtectedInterface + { + private android.os.IBinder mRemote; + Proxy(android.os.IBinder remote) + { + mRemote = remote; + } + @Override public android.os.IBinder asBinder() + { + return mRemote; + } + public java.lang.String getInterfaceDescriptor() + { + return DESCRIPTOR; + } + @Override public void Method1() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_Method1, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void Method2() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_Method2, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + } + static final int TRANSACTION_Method1 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0); + /** Helper method to enforce permissions for Method1 */ + protected void Method1_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source); + } + static final int TRANSACTION_Method2 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1); + /** Helper method to enforce permissions for Method2 */ + protected void Method2_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source); + } + /** @hide */ + public int getMaxTransactionId() + { + return 1; + } + } + + @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION) + public void Method1() throws android.os.RemoteException; + @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION) + public void Method2() throws android.os.RemoteException; + } + """ + ).indented(), + *stubs + ) + .run() + .expectClean() + } + + /* Stubs */ + + private val manifestPermissionStub: TestFile = java( + """ + package android.Manifest; + class permission { + public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE"; + public static final String INTERNET = "android.permission.INTERNET"; + } + """ + ).indented() + + private val enforcePermissionAnnotationStub: TestFile = java( + """ + package android.annotation; + public @interface EnforcePermission {} + """ + ).indented() + + private val stubs = arrayOf(manifestPermissionStub, enforcePermissionAnnotationStub) +} diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt index 3c1d1e8e8a59b..618bbcc99656a 100644 --- a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt +++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt @@ -189,6 +189,28 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \ 1 errors, 0 warnings""".addLineContinuation()) } + fun testDetectIssuesMissingAnnotationOnMethodWhenClassIsCalledDefault() { + lint().files(java( + """ + package test.pkg; + public class Default extends IFooMethod.Stub { + public void testMethod() {} + } + """).indented(), + *stubs + ) + .run() + .expect( + """ + src/test/pkg/Default.java:3: Error: The method Default.testMethod \ + overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same annotation must be used on Default.testMethod [MissingEnforcePermissionAnnotation] + public void testMethod() {} + ~~~~~~~~~~ + 1 errors, 0 warnings + """.addLineContinuation() + ) + } + /* Stubs */ // A service with permission annotation on the class. diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorCodegenTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorCodegenTest.kt new file mode 100644 index 0000000000000..5a63bb4084d21 --- /dev/null +++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorCodegenTest.kt @@ -0,0 +1,557 @@ +/* + * Copyright (C) 2022 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.android.lint.aidl + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestFile +import com.android.tools.lint.checks.infrastructure.TestLintTask +import com.android.tools.lint.checks.infrastructure.TestMode +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue + +@Suppress("UnstableApiUsage") +class EnforcePermissionHelperDetectorCodegenTest : LintDetectorTest() { + override fun getDetector(): Detector = EnforcePermissionHelperDetector() + + override fun getIssues(): List = listOf( + EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER + ) + + override fun lint(): TestLintTask = super.lint().allowMissingSdk(true) + + fun test_generated_IProtected() { + lint().testModes(TestMode.DEFAULT).files( + java( + """ + /* + * This file is auto-generated. DO NOT MODIFY. + */ + package android.aidl.tests.permission; + public interface IProtected extends android.os.IInterface + { + /** Default implementation for IProtected. */ + public static class Default implements android.aidl.tests.permission.IProtected + { + @Override public void PermissionProtected() throws android.os.RemoteException + { + } + @Override public void MultiplePermissionsAll() throws android.os.RemoteException + { + } + @Override public void MultiplePermissionsAny() throws android.os.RemoteException + { + } + @Override public void NonManifestPermission() throws android.os.RemoteException + { + } + // Used by the integration tests to dynamically set permissions that are considered granted. + @Override public void SetGranted(java.util.List permissions) throws android.os.RemoteException + { + } + @Override + public android.os.IBinder asBinder() { + return null; + } + } + /** Local-side IPC implementation stub class. */ + public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtected + { + private final android.os.PermissionEnforcer mEnforcer; + /** Construct the stub using the Enforcer provided. */ + public Stub(android.os.PermissionEnforcer enforcer) + { + this.attachInterface(this, DESCRIPTOR); + if (enforcer == null) { + throw new IllegalArgumentException("enforcer cannot be null"); + } + mEnforcer = enforcer; + } + @Deprecated + /** Default constructor. */ + public Stub() { + this(android.os.PermissionEnforcer.fromContext( + android.app.ActivityThread.currentActivityThread().getSystemContext())); + } + /** + * Cast an IBinder object into an android.aidl.tests.permission.IProtected interface, + * generating a proxy if needed. + */ + public static android.aidl.tests.permission.IProtected asInterface(android.os.IBinder obj) + { + if ((obj==null)) { + return null; + } + android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR); + if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtected))) { + return ((android.aidl.tests.permission.IProtected)iin); + } + return new android.aidl.tests.permission.IProtected.Stub.Proxy(obj); + } + @Override public android.os.IBinder asBinder() + { + return this; + } + /** @hide */ + public static java.lang.String getDefaultTransactionName(int transactionCode) + { + switch (transactionCode) + { + case TRANSACTION_PermissionProtected: + { + return "PermissionProtected"; + } + case TRANSACTION_MultiplePermissionsAll: + { + return "MultiplePermissionsAll"; + } + case TRANSACTION_MultiplePermissionsAny: + { + return "MultiplePermissionsAny"; + } + case TRANSACTION_NonManifestPermission: + { + return "NonManifestPermission"; + } + case TRANSACTION_SetGranted: + { + return "SetGranted"; + } + default: + { + return null; + } + } + } + /** @hide */ + public java.lang.String getTransactionName(int transactionCode) + { + return this.getDefaultTransactionName(transactionCode); + } + @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException + { + java.lang.String descriptor = DESCRIPTOR; + if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) { + data.enforceInterface(descriptor); + } + switch (code) + { + case INTERFACE_TRANSACTION: + { + reply.writeString(descriptor); + return true; + } + } + switch (code) + { + case TRANSACTION_PermissionProtected: + { + this.PermissionProtected(); + reply.writeNoException(); + break; + } + case TRANSACTION_MultiplePermissionsAll: + { + this.MultiplePermissionsAll(); + reply.writeNoException(); + break; + } + case TRANSACTION_MultiplePermissionsAny: + { + this.MultiplePermissionsAny(); + reply.writeNoException(); + break; + } + case TRANSACTION_NonManifestPermission: + { + this.NonManifestPermission(); + reply.writeNoException(); + break; + } + case TRANSACTION_SetGranted: + { + java.util.List _arg0; + _arg0 = data.createStringArrayList(); + data.enforceNoDataAvail(); + this.SetGranted(_arg0); + reply.writeNoException(); + break; + } + default: + { + return super.onTransact(code, data, reply, flags); + } + } + return true; + } + private static class Proxy implements android.aidl.tests.permission.IProtected + { + private android.os.IBinder mRemote; + Proxy(android.os.IBinder remote) + { + mRemote = remote; + } + @Override public android.os.IBinder asBinder() + { + return mRemote; + } + public java.lang.String getInterfaceDescriptor() + { + return DESCRIPTOR; + } + @Override public void PermissionProtected() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_PermissionProtected, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void MultiplePermissionsAll() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAll, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void MultiplePermissionsAny() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAny, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void NonManifestPermission() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_NonManifestPermission, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + // Used by the integration tests to dynamically set permissions that are considered granted. + @Override public void SetGranted(java.util.List permissions) throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + _data.writeStringList(permissions); + boolean _status = mRemote.transact(Stub.TRANSACTION_SetGranted, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + } + static final int TRANSACTION_PermissionProtected = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0); + /** Helper method to enforce permissions for PermissionProtected */ + protected void PermissionProtected_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, source); + } + static final int TRANSACTION_MultiplePermissionsAll = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1); + /** Helper method to enforce permissions for MultiplePermissionsAll */ + protected void MultiplePermissionsAll_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source); + } + static final int TRANSACTION_MultiplePermissionsAny = (android.os.IBinder.FIRST_CALL_TRANSACTION + 2); + /** Helper method to enforce permissions for MultiplePermissionsAny */ + protected void MultiplePermissionsAny_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermissionAnyOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source); + } + static final int TRANSACTION_NonManifestPermission = (android.os.IBinder.FIRST_CALL_TRANSACTION + 3); + /** Helper method to enforce permissions for NonManifestPermission */ + protected void NonManifestPermission_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, source); + } + static final int TRANSACTION_SetGranted = (android.os.IBinder.FIRST_CALL_TRANSACTION + 4); + /** @hide */ + public int getMaxTransactionId() + { + return 4; + } + } + + @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + public void PermissionProtected() throws android.os.RemoteException; + @android.annotation.EnforcePermission(allOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}) + public void MultiplePermissionsAll() throws android.os.RemoteException; + @android.annotation.EnforcePermission(anyOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}) + public void MultiplePermissionsAny() throws android.os.RemoteException; + @android.annotation.EnforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) + public void NonManifestPermission() throws android.os.RemoteException; + // Used by the integration tests to dynamically set permissions that are considered granted. + @android.annotation.RequiresNoPermission + public void SetGranted(java.util.List permissions) throws android.os.RemoteException; + } + """ + ).indented(), + *stubs + ) + .run() + .expectClean() + } + + fun test_generated_IProtectedInterface() { + lint().files( + java( + """ + /* + * This file is auto-generated. DO NOT MODIFY. + */ + package android.aidl.tests.permission; + public interface IProtectedInterface extends android.os.IInterface + { + /** Default implementation for IProtectedInterface. */ + public static class Default implements android.aidl.tests.permission.IProtectedInterface + { + @Override public void Method1() throws android.os.RemoteException + { + } + @Override public void Method2() throws android.os.RemoteException + { + } + @Override + public android.os.IBinder asBinder() { + return null; + } + } + /** Local-side IPC implementation stub class. */ + public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtectedInterface + { + private final android.os.PermissionEnforcer mEnforcer; + /** Construct the stub using the Enforcer provided. */ + public Stub(android.os.PermissionEnforcer enforcer) + { + this.attachInterface(this, DESCRIPTOR); + if (enforcer == null) { + throw new IllegalArgumentException("enforcer cannot be null"); + } + mEnforcer = enforcer; + } + @Deprecated + /** Default constructor. */ + public Stub() { + this(android.os.PermissionEnforcer.fromContext( + android.app.ActivityThread.currentActivityThread().getSystemContext())); + } + /** + * Cast an IBinder object into an android.aidl.tests.permission.IProtectedInterface interface, + * generating a proxy if needed. + */ + public static android.aidl.tests.permission.IProtectedInterface asInterface(android.os.IBinder obj) + { + if ((obj==null)) { + return null; + } + android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR); + if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtectedInterface))) { + return ((android.aidl.tests.permission.IProtectedInterface)iin); + } + return new android.aidl.tests.permission.IProtectedInterface.Stub.Proxy(obj); + } + @Override public android.os.IBinder asBinder() + { + return this; + } + /** @hide */ + public static java.lang.String getDefaultTransactionName(int transactionCode) + { + switch (transactionCode) + { + case TRANSACTION_Method1: + { + return "Method1"; + } + case TRANSACTION_Method2: + { + return "Method2"; + } + default: + { + return null; + } + } + } + /** @hide */ + public java.lang.String getTransactionName(int transactionCode) + { + return this.getDefaultTransactionName(transactionCode); + } + @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException + { + java.lang.String descriptor = DESCRIPTOR; + if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) { + data.enforceInterface(descriptor); + } + switch (code) + { + case INTERFACE_TRANSACTION: + { + reply.writeString(descriptor); + return true; + } + } + switch (code) + { + case TRANSACTION_Method1: + { + this.Method1(); + reply.writeNoException(); + break; + } + case TRANSACTION_Method2: + { + this.Method2(); + reply.writeNoException(); + break; + } + default: + { + return super.onTransact(code, data, reply, flags); + } + } + return true; + } + private static class Proxy implements android.aidl.tests.permission.IProtectedInterface + { + private android.os.IBinder mRemote; + Proxy(android.os.IBinder remote) + { + mRemote = remote; + } + @Override public android.os.IBinder asBinder() + { + return mRemote; + } + public java.lang.String getInterfaceDescriptor() + { + return DESCRIPTOR; + } + @Override public void Method1() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_Method1, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + @Override public void Method2() throws android.os.RemoteException + { + android.os.Parcel _data = android.os.Parcel.obtain(asBinder()); + android.os.Parcel _reply = android.os.Parcel.obtain(); + try { + _data.writeInterfaceToken(DESCRIPTOR); + boolean _status = mRemote.transact(Stub.TRANSACTION_Method2, _data, _reply, 0); + _reply.readException(); + } + finally { + _reply.recycle(); + _data.recycle(); + } + } + } + static final int TRANSACTION_Method1 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0); + /** Helper method to enforce permissions for Method1 */ + protected void Method1_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source); + } + static final int TRANSACTION_Method2 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1); + /** Helper method to enforce permissions for Method2 */ + protected void Method2_enforcePermission() throws SecurityException { + android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null); + mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source); + } + /** @hide */ + public int getMaxTransactionId() + { + return 1; + } + } + + @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION) + public void Method1() throws android.os.RemoteException; + @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION) + public void Method2() throws android.os.RemoteException; + } + """ + ).indented(), + *stubs + ) + .run() + .expectClean() + } + + /* Stubs */ + + private val manifestPermissionStub: TestFile = java( + """ + package android.Manifest; + class permission { + public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE"; + public static final String INTERNET = "android.permission.INTERNET"; + } + """ + ).indented() + + private val enforcePermissionAnnotationStub: TestFile = java( + """ + package android.annotation; + public @interface EnforcePermission {} + """ + ).indented() + + private val stubs = arrayOf(manifestPermissionStub, enforcePermissionAnnotationStub) +} diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt index 31e484628a041..4799184b0e239 100644 --- a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt +++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt @@ -150,6 +150,31 @@ class EnforcePermissionHelperDetectorTest : LintDetectorTest() { .expectClean() } + fun testInterfaceDefaultMethod_wouldStillReport() { + lint().files( + java( + """ + public interface IProtected extends android.os.IInterface { + @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + default void PermissionProtected() throws android.os.RemoteException { + String foo = "bar"; + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/IProtected.java:2: Error: Method must start with super.PermissionProtected_enforcePermission() [MissingEnforcePermissionHelper] + @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + ^ + 1 errors, 0 warnings + """ + ) + } + companion object { val stubs = arrayOf(aidlStub, contextStub, binderStub) }