From 7d90aacd9b89beb098aa4f7c0993634e7b2c9d93 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Fri, 5 Feb 2021 17:13:31 -0800 Subject: Fix permission check bypass in IMMS#dump() This is a follow up CL to our previous CL [1], which enabled IME tracing result to be automatically included into the bugreport. In the previous CL, we checked android.Manifest.permission.DUMP in IMMS#doDump() not in IMMS#dump(), which means there was a chance that some priviledged operations could be triggered by a user-mode application. So far the only actual imprecation is that a user-mode process can restart an on-going IME trace session, but in general such an operation should be allowed only to processes that have android.Manifest.permission.DUMP or shell access. With this CL, all the operations inside IMMS#dump() will require android.Manifest.permission.DUMP again as we did before the previous CL was submitted. Other than that, there should be no behavior change in this CL. This is a preparation to fix another unintentional behavior change (Bug 177462676), which was also accidentally introduced in the same CL. [1]: Ie87eb8423e2bb70f28c330983d45b95e2e07062d ac24994aaa5bd1f70608cb8c72b93b5ae00d90eb Bug: 154348613 Bug: 167948910 Bug: 177462676 Test: Manually done as follows. 1. adb shell ime tracing start 2. adb logcat -s imeTracing:* Make sure IME tracing started. 3. adb shell am start -a android.intent.action.DIAL 4. adb bugreport bugreport.zip 5. adb logcat -s imeTracing:* Make sure IME tracing is stopped then restarted. 6. unzip -v bugreport.zip | grep ime_trace_clients.pb Make sure "ime_trace_clients.pb" is included. Change-Id: I3de965bc2993df67b046f3e6c09ece11047d227e --- .../com/android/server/inputmethod/InputMethodManagerService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index d8e124a00104..78588c9bc6da 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -5227,6 +5227,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { + if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return; + boolean asProto = false; for (int argIndex = 0; argIndex < args.length; argIndex++) { if (args[argIndex].equals(PROTO_ARG)) { @@ -5249,8 +5251,6 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } private void doDump(FileDescriptor fd, PrintWriter pw, String[] args, boolean useProto) { - if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return; - if (useProto) { final ProtoOutputStream proto = new ProtoOutputStream(fd); dumpDebug(proto, InputMethodManagerServiceTraceProto.INPUT_METHOD_MANAGER_SERVICE); -- cgit v1.2.3-59-g8ed1b