CVE-2023-20971 AI-generated analysis

platform/frameworks/base, services/core PermissionManagerServiceImpl (permission registry)

AI-generated: written from the bulletin and the fixing diffs by claude-fable-5-1, then checked against the code by claude-opus-5-5 (4 corrections). Verify against the commit before relying on it.

Adds a missing early return in PermissionManagerServiceImpl.removePermission() so that a non-dynamic permission is no longer removed from the registry after the "Not allowed to modify non-dynamic permission" check fails.

Root cause

In services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java, removePermission(String permName) looks up the Permission from mRegistry and, if !bp.isDynamic(), logs Slog.wtf("Not allowed to modify non-dynamic permission ...") but then falls through to mRegistry.removePermission(permName) and mPackageManagerInt.writeSettings(false). Per the commit message only dynamic permissions are supposed to be removable, but the code let a caller that passes mRegistry.enforcePermissionTree(permName, callingUid) remove a non-dynamic permission from the registry and persist that change.

How the fix works

• PermissionManagerServiceImpl.java:removePermission - a single `return;` is added inside the `if (!bp.isDynamic())` branch, after the Slog.wtf log, so the method exits (still inside synchronized (mLock)) before mRegistry.removePermission(permName) is called and before mPackageManagerInt.writeSettings(false) runs.
• The existing pre-checks are unchanged: instant apps are still rejected with SecurityException, mRegistry.enforcePermissionTree(permName, callingUid) is still enforced, and a null Permission still returns early.

Completeness

• The fix covers only removePermission(); it makes the non-dynamic branch actually terminate the operation.
• The sibling addPermission() path in the same file already throws SecurityException for a non-dynamic existing permission (lines 667-669 in the r29 context), so no equivalent gap is visible there.
• The non-dynamic case still logs Slog.wtf and returns silently rather than throwing; the code retains the comment 'TODO: switch this back to SecurityException', so callers receive no error signal.

Behavior changes

• removePermission() on a non-dynamic permission is now a no-op (after a Slog.wtf log): the permission stays in mRegistry and settings are not rewritten.
• No new exception or error code is introduced; the caller cannot distinguish the rejected call from a successful one.

Per major

• Android 14: same fix: commit 6c22d6c1e696 (Change-Id Ie2f43663bc71a06ffadb868d2d0eea5ee78f76e5) in android-security-14.0.0_r9; at android-security-14.0.0_r29 all 3 lines of the patched region are present.
• Android 15: not affected (not listed by the bulletin); at android-security-15.0.0_r17 the patched region (3/3 lines) is present.
• Android 16: not affected (not listed by the bulletin); at android-security-16.0.0_r8 the patched region (3/3 lines) is present.
• Android 17: not affected (not listed by the bulletin); services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java is absent at android-security-17.0.0_r1.

Unknowns

• The material does not show the fix commits for Android 12, 12L and 13 that the bulletin lists as affected.
• The material does not show the Binder-facing callers of removePermission() or any test accompanying the change (commit says Test: manual).
• The exact semantics of mRegistry.enforcePermissionTree() are not shown.

Back to August 2024 · Findings