From b8d240fa3f96b7b4ea35dd271beda789044d63ab Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Wed, 26 Dec 2018 10:03:11 -0800 Subject: [PATCH] Lock down IInputMethodManager#shellCommand() based on caller UID This is part of our on-going effort to review caller verifications in InputMethodManagerService (IMMS). In Android P, IMMS started relying on IBinder#shellCommand() to implement 'adb shell ime' command [1]. When handling incoming request, following caller verifications are used depending on the command type. * IMMS#calledFromValidUserLocked() * This can be bypassed with INTERACT_ACROSS_USERS_FULL permission * WRITE_SECURE_SETTINGS permission From the viewpoint of caller verification, this is basically the same as how commands like 'adb shell ime' were handled before IBinder#shellCommand(). What this CL aims to do is adding one more foolproof to this protocol. Given that all commands exposed via IInputMethodManager#shellCommand() are intended to be used only from "shell" environment, it is most likely safe to reject any request from non-shell users. With this additional restriction, even if some caller verification was accidentally missed in those shell commands such a security hole would not be exposed to random applications. [1]: I9a2dbbf1d4494addbe22c82e2c416eedc4d585f2 926488d70d09baefee0489537b2915602deaeebf Bug: 34886274 Fix: 121989657 Test: Following commands still work, before/after "adb shell root" * adb shell ime * adb shell ime list * adb shell ime set com.android.inputmethod.latin/.LatinIME * adb shell cmd input_method * adb shell cmd input_method refresh_debug_properties * adb shell dumpsys input_method Test: atest CtsInputMethodTestCases CtsInputMethodServiceHostTestCases Change-Id: If87189563ccaacd4f9c666bab4f9ad08a9343084 --- .../InputMethodManagerService.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index dd4afaf903593..292aae81b421f 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -179,6 +179,7 @@ import java.nio.charset.StandardCharsets; import java.security.InvalidParameterException; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.List; @@ -4587,6 +4588,25 @@ public class InputMethodManagerService extends IInputMethodManager.Stub @Nullable FileDescriptor err, @NonNull String[] args, @Nullable ShellCallback callback, @NonNull ResultReceiver resultReceiver) throws RemoteException { + final int callingUid = Binder.getCallingUid(); + // Reject any incoming calls from non-shell users, including ones from the system user. + if (callingUid != Process.ROOT_UID && callingUid != Process.SHELL_UID) { + // Note that Binder#onTransact() will automatically close "in", "out", and "err" when + // returned from this method, hence there is no need to close those FDs. + // "resultReceiver" is the only thing that needs to be taken care of here. + if (resultReceiver != null) { + resultReceiver.send(ShellCommandResult.FAILURE, null); + } + final String errorMsg = "InputMethodManagerService does not support shell commands from" + + " non-shell users. callingUid=" + callingUid + + " args=" + Arrays.toString(args); + if (Process.isCoreUid(callingUid)) { + // Let's not crash the calling process if the caller is one of core components. + Slog.e(TAG, errorMsg); + return; + } + throw new SecurityException(errorMsg); + } new ShellCommandImpl(this).exec( this, in, out, err, args, callback, resultReceiver); }