Merge "[CallingIdentityTokenDetector] New issue - result of... (28f1399f) · Commits · e / os / android_frameworks_base

tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt

+15 −14

Original line number	Diff line number	Diff line
		@@ -32,11 +32,12 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
		CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
		CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
		CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
		CallingIdentityTokenDetector.ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE,
		CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED,
		EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
		EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
		SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
		PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
		PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS
		)

		override val api: Int

tools/lint/checks/src/main/java/com/google/android/lint/CallingIdentityTokenDetector.kt

+223 −152

Original line number	Diff line number	Diff line
		@@ -33,6 +33,7 @@ import org.jetbrains.uast.UBlockExpression
		import org.jetbrains.uast.UCallExpression
		import org.jetbrains.uast.UDeclarationsExpression
		import org.jetbrains.uast.UElement
		import org.jetbrains.uast.UIfExpression
		import org.jetbrains.uast.ULocalVariable
		import org.jetbrains.uast.USimpleNameReferenceExpression
		import org.jetbrains.uast.UTryExpression
		@@ -144,16 +145,40 @@ class CallingIdentityTokenDetector : Detector(), SourceCodeScanner {
		)
		}

		/**
		* For every method():
		* - Checks use of caller-aware methods issue
		* For every call of Binder.restoreCallingIdentity(token):
		* - Checks for restoreCallingIdentity() not in the finally block issue
		* - Removes token from tokensMap if token is within the scope of the method
		*/
		override fun visitCallExpression(node: UCallExpression) {
		when {
		isMethodCall(node, Method.BINDER_CLEAR_CALLING_IDENTITY) -> {
		checkClearCallingIdentityCall(node)
		}
		isMethodCall(node, Method.BINDER_RESTORE_CALLING_IDENTITY) -> {
		checkRestoreCallingIdentityCall(node)
		}
		isCallerAwareMethod(node) -> checkCallerAwareMethod(node)
		}
		}

		private fun checkClearCallingIdentityCall(node: UCallExpression) {
		var firstNonQualifiedParent = getFirstNonQualifiedParent(node)
		// if the call expression is inside a ternary, and the ternary is assigned
		// to a variable, then we are still technically assigning
		// any result of clearCallingIdentity to a variable
		if (firstNonQualifiedParent is UIfExpression && firstNonQualifiedParent.isTernary) {
		firstNonQualifiedParent = firstNonQualifiedParent.uastParent
		}
		if (firstNonQualifiedParent !is ULocalVariable) {
		context.report(
		ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE,
		context.getLocation(node),
		getIncidentMessageResultOfClearIdentityCallNotStoredInVariable(
		node.getQualifiedParentOrThis().asRenderString()
		)
		)
		}
		}

		private fun checkCallerAwareMethod(node: UCallExpression) {
		val token = findFirstTokenInScope(node)
		if (isCallerAwareMethod(node) && token != null) {
		if (token != null) {
		context.report(
		ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
		context.getLocation(node),
		@@ -162,11 +187,15 @@ class CallingIdentityTokenDetector : Detector(), SourceCodeScanner {
		node.asRenderString()
		)
		)
		return
		}
		if (!isMethodCall(node, Method.BINDER_RESTORE_CALLING_IDENTITY)) return
		val first = node.valueArguments[0].skipParenthesizedExprDown()
		val arg = first as? USimpleNameReferenceExpression ?: return
		}

		/**
		* - Checks for restoreCallingIdentity() not in the finally block issue
		* - Removes token from tokensMap if token is within the scope of the method
		*/
		private fun checkRestoreCallingIdentityCall(node: UCallExpression) {
		val arg = node.valueArguments[0] as? USimpleNameReferenceExpression ?: return
		val variableName = arg.identifier
		val originalScope = tokensMap[variableName]?.scope ?: return
		val psi = arg.sourcePsi ?: return
		@@ -174,15 +203,13 @@ class CallingIdentityTokenDetector : Detector(), SourceCodeScanner {
		// token declaration. If not within the scope, no action is needed because the token is
		// irrelevant i.e. not in the same scope or was not declared with clearCallingIdentity()
		if (!PsiSearchScopeUtil.isInScope(originalScope, psi)) return
		// - We do not report "restore identity call not in finally" issue when there is no
		// We do not report "restore identity call not in finally" issue when there is no
		// finally block because that case is already handled by "clear identity call not
		// followed by try-finally" issue
		// - UCallExpression can be a child of UQualifiedReferenceExpression, i.e.
		// receiver.selector, so to get the call's immediate parent we need to get the topmost
		// parent qualified reference expression and access its parent
		if (tokensMap[variableName]?.finallyBlock != null &&
		skipParenthesizedExprUp(node.getQualifiedParentOrThis().uastParent) !=
		tokensMap[variableName]?.finallyBlock) {
		getFirstNonQualifiedParent(node) !=
		tokensMap[variableName]?.finallyBlock
		) {
		context.report(
		ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
		context.getLocation(node),
		@@ -192,6 +219,13 @@ class CallingIdentityTokenDetector : Detector(), SourceCodeScanner {
		tokensMap.remove(variableName)
		}

		private fun getFirstNonQualifiedParent(expression: UCallExpression): UElement? {
		// UCallExpression can be a child of UQualifiedReferenceExpression, i.e.
		// receiver.selector, so to get the call's immediate parent we need to get the topmost
		// parent qualified reference expression and access its parent
		return skipParenthesizedExprUp(expression.getQualifiedParentOrThis().uastParent)
		}

		private fun isCallerAwareMethod(expression: UCallExpression): Boolean =
		callerAwareMethods.any { method -> isMethodCall(expression, method) }

		@@ -573,5 +607,42 @@ class CallingIdentityTokenDetector : Detector(), SourceCodeScanner {
		"identity instead of the caller's. Either explicitly query your own identity or " +
		"move it after restoring the identity with " +
		"`Binder.restoreCallingIdentity($variableName)`."

		/** Issue: Result of Binder.clearCallingIdentity() is not stored in a variable */
		@JvmField
		val ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE: Issue = Issue.create(
		id = "ResultOfClearIdentityCallNotStoredInVariable",
		briefDescription = "Result of Binder.clearCallingIdentity() is not stored in a " +
		"variable",
		explanation = """
		You cleared the original calling identity with \
		`Binder.clearCallingIdentity()`, but did not store the result of the method \
		call in a variable. You need to store the result in a variable and restore it later.

		Use the following pattern for running operations with your own identity:

		```
		final long token = Binder.clearCallingIdentity();
		try {
		// Code using your own identity
		} finally {
		Binder.restoreCallingIdentity(token);
		}
		```
		""",
		category = Category.SECURITY,
		priority = 6,
		severity = Severity.WARNING,
		implementation = Implementation(
		CallingIdentityTokenDetector::class.java,
		Scope.JAVA_FILE_SCOPE
		)
		)

		private fun getIncidentMessageResultOfClearIdentityCallNotStoredInVariable(
		methodName: String
		): String = "You cleared the original identity with `$methodName` but did not store the " +
		"result in a variable. You need to store the result in a variable and restore it " +
		"later."
		}
		}

tools/lint/checks/src/test/java/com/google/android/lint/CallingIdentityTokenDetectorTest.kt

+147 −94

Original line number	Diff line number	Diff line
		@@ -32,7 +32,8 @@ class CallingIdentityTokenDetectorTest : LintDetectorTest() {
		CallingIdentityTokenDetector.ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
		CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
		CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
		CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY
		CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
		CallingIdentityTokenDetector.ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE
		)

		override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
		@@ -62,6 +63,13 @@ class CallingIdentityTokenDetectorTest : LintDetectorTest() {
		} finally {
		restoreCallingIdentity(token3);
		}
		final Long token4 = true ? Binder.clearCallingIdentity() : null;
		try {
		} finally {
		if (token4 != null) {
		restoreCallingIdentity(token4);
		}
		}
		}
		}
		"""
		@@ -739,6 +747,51 @@ class CallingIdentityTokenDetectorTest : LintDetectorTest() {
		)
		}

		/** Result of Binder.clearCallingIdentity() is not stored in a variable issue tests */

		fun testDetectsResultOfClearIdentityCallNotStoredInVariable() {
		lint().files(
		java(
		"""
		package test.pkg;
		import android.os.Binder;
		public class TestClass1 extends Binder {
		private void testMethod() {
		Binder.clearCallingIdentity();
		android.os.Binder.clearCallingIdentity();
		clearCallingIdentity();
		}
		}
		"""
		).indented(),
		*stubs
		)
		.run()
		.expect(
		"""
		src/test/pkg/TestClass1.java:5: Warning: You cleared the original identity \
		with Binder.clearCallingIdentity() but did not store the result in a \
		variable. You need to store the result in a variable and restore it later. \
		[ResultOfClearIdentityCallNotStoredInVariable]
		Binder.clearCallingIdentity();
		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
		src/test/pkg/TestClass1.java:6: Warning: You cleared the original identity \
		with android.os.Binder.clearCallingIdentity() but did not store the result \
		in a variable. You need to store the result in a variable and restore it \
		later. [ResultOfClearIdentityCallNotStoredInVariable]
		android.os.Binder.clearCallingIdentity();
		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
		src/test/pkg/TestClass1.java:7: Warning: You cleared the original identity \
		with clearCallingIdentity() but did not store the result in a variable. \
		You need to store the result in a variable and restore it later. \
		[ResultOfClearIdentityCallNotStoredInVariable]
		clearCallingIdentity();
		~~~~~~~~~~~~~~~~~~~~~~
		0 errors, 3 warnings
		""".addLineContinuation()
		)
		}

		/** Stubs for classes used for testing */

		private val binderStub: TestFile = java(