Merge "SimpleManualPermissionEnforcementDetector: minor cleanup"

This commit is contained in:
Matt Gilbride
2022-12-02 15:23:08 +00:00
committed by Android (Google) Code Review
5 changed files with 108 additions and 96 deletions

View File

@@ -40,7 +40,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
SimpleManualPermissionEnforcementDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
RegisterReceiverFlagDetector.ISSUE_RECEIVER_EXPORTED_FLAG,

View File

@@ -31,7 +31,7 @@ class AndroidGlobalIssueRegistry : IssueRegistry() {
EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
SimpleManualPermissionEnforcementDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
)
override val api: Int

View File

@@ -17,6 +17,7 @@
package com.google.android.lint.aidl
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.LintFix
import com.android.tools.lint.detector.api.Location
import com.android.tools.lint.detector.api.getUMethod
import com.google.android.lint.hasPermissionNameAnnotation
@@ -38,11 +39,34 @@ data class EnforcePermissionFix(
val locations: List<Location>,
val permissionNames: List<String>
) {
val annotation: String
fun toLintFix(annotationLocation: Location): LintFix {
val removeFixes = this.locations.map {
LintFix.create()
.replace()
.reformat(true)
.range(it)
.with("")
.autoFix()
.build()
}
val annotateFix = LintFix.create()
.annotate(this.annotation)
.range(annotationLocation)
.autoFix()
.build()
return LintFix.create().composite(annotateFix, *removeFixes.toTypedArray())
}
private val annotation: String
get() {
val quotedPermissions = permissionNames.joinToString(", ") { """"$it"""" }
val annotationParameter =
if (permissionNames.size > 1) "allOf={$quotedPermissions}" else quotedPermissions
if (permissionNames.size > 1) "allOf={$quotedPermissions}"
else quotedPermissions
return "@$ANNOTATION_ENFORCE_PERMISSION($annotationParameter)"
}

View File

@@ -28,6 +28,7 @@ import org.jetbrains.uast.UElement
import org.jetbrains.uast.UIfExpression
import org.jetbrains.uast.UMethod
import org.jetbrains.uast.UQualifiedReferenceExpression
import org.jetbrains.uast.skipParenthesizedExprDown
/**
* Looks for methods implementing generated AIDL interface stubs
@@ -44,29 +45,16 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
interfaceName: String,
body: UBlockExpression
) {
val fix = accumulateSimplePermissionCheckFixes(body, context) ?: return
val javaRemoveFixes = fix.locations.map {
fix()
.replace()
.reformat(true)
.range(it)
.with("")
.autoFix()
.build()
}
val javaAnnotateFix = fix()
.annotate(fix.annotation)
.range(context.getLocation(node))
.autoFix()
.build()
val enforcePermissionFix = accumulateSimplePermissionCheckFixes(body, context) ?: return
val lintFix = enforcePermissionFix.toLintFix(context.getLocation(node))
val message =
"$interfaceName permission check can be converted to @EnforcePermission annotation"
context.report(
ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
fix.locations.last(),
"$interfaceName permission check can be converted to @EnforcePermission annotation",
fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix)
ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
enforcePermissionFix.locations.last(),
message,
lintFix
)
}
@@ -89,7 +77,8 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
EnforcePermissionFix? {
val singleFixes = mutableListOf<EnforcePermissionFix>()
for (expression in methodBody.expressions) {
singleFixes.add(getPermissionCheckFix(expression, context) ?: break)
singleFixes.add(getPermissionCheckFix(expression.skipParenthesizedExprDown(), context)
?: break)
}
return when (singleFixes.size) {
0 -> null
@@ -133,7 +122,7 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
""".trimIndent()
@JvmField
val ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION = Issue.create(
val ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT = Issue.create(
id = "SimpleManualPermissionEnforcement",
briefDescription = "Manual permission check can be @EnforcePermission annotation",
explanation = EXPLANATION,

View File

@@ -18,7 +18,6 @@ package com.google.android.lint.aidl
import com.android.tools.lint.checks.infrastructure.LintDetectorTest
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
@@ -27,7 +26,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
override fun getDetector(): Detector = SimpleManualPermissionEnforcementDetector()
override fun getIssues(): List<Issue> = listOf(
SimpleManualPermissionEnforcementDetector
.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION
.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT
)
override fun lint(): TestLintTask = super.lint().allowMissingSdk()
@@ -36,15 +35,15 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
}
"""
).indented(),
*stubs
@@ -73,18 +72,18 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
}
};
}
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
}
};
}
"""
).indented(),
*stubs
@@ -114,16 +113,16 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
}
public class Foo extends ITest.Stub {
private Context mContext;
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
}
}
"""
).indented(),
*stubs,
@@ -153,20 +152,20 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
mContext.enforceCallingOrSelfPermission(
"android.permission.WRITE_CONTACTS", "foo");
}
};
}
import android.content.Context;
import android.test.ITest;
public class Foo {
private Context mContext;
private ITest itest = new ITest.Stub() {
@Override
public void test() throws android.os.RemoteException {
mContext.enforceCallingOrSelfPermission(
"android.permission.READ_CONTACTS", "foo");
mContext.enforceCallingOrSelfPermission(
"android.permission.WRITE_CONTACTS", "foo");
}
};
}
"""
).indented(),
*stubs
@@ -198,16 +197,16 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
lint().files(
java(
"""
import android.os.Binder;
import android.test.ITest;
public class Foo extends ITest.Stub {
private mContext Context;
@Override
public void test() throws android.os.RemoteException {
long uid = Binder.getCallingUid();
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
import android.os.Binder;
import android.test.ITest;
public class Foo extends ITest.Stub {
private mContext Context;
@Override
public void test() throws android.os.RemoteException {
long uid = Binder.getCallingUid();
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
}
"""
).indented(),
*stubs
@@ -217,25 +216,25 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
}
fun testPermissionHelper() {
lint().skipTestModes(TestMode.PARENTHESIZED).files(
lint().files(
java(
"""
import android.content.Context;
import android.test.ITest;
import android.content.Context;
import android.test.ITest;
public class Foo extends ITest.Stub {
private Context mContext;
public class Foo extends ITest.Stub {
private Context mContext;
@android.content.pm.PermissionMethod
private void helper() {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
@Override
public void test() throws android.os.RemoteException {
helper();
}
@android.content.pm.PermissionMethod
private void helper() {
mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
}
@Override
public void test() throws android.os.RemoteException {
helper();
}
}
"""
).indented(),
*stubs
@@ -261,7 +260,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
}
fun testPermissionHelperAllOf() {
lint().skipTestModes(TestMode.PARENTHESIZED).files(
lint().files(
java(
"""
import android.content.Context;
@@ -309,7 +308,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
fun testPermissionHelperNested() {
lint().skipTestModes(TestMode.PARENTHESIZED).files(
lint().files(
java(
"""
import android.content.Context;