Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b93712f6 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Tighten up Binder.clearCallingIdentity() usage. 

This is a third CL in a chain that adjusts existing malformed code
to follow AndroidFrameworkBinderIdentity best-practices.

Specifically, if a thread clears an identity they need to restore it
to avoid obscure security vulnerabilities.  In addition, the relevant
"try" block must start immediately after the identity is cleared to
ensure that its restored if/when any exceptions are thrown.

Bug: 155703208
Test: make
Exempt-From-Owner-Approval: trivial refactoring
Change-Id: I74cb958b68d55a647547aae21baff6ddc364859b
parent b381789f

core/java/android/app/backup/BackupAgent.java

+15 −13
Original line number Diff line number Diff line
@@ -1054,14 +1054,15 @@ public abstract class BackupAgent extends ContextWrapper { 
                long quotaBytes,

                IBackupCallback callbackBinder,

                int transportFlags) throws RemoteException {

            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();



            if (DEBUG) Log.v(TAG, "doBackup() invoked");



            BackupDataOutput output = new BackupDataOutput(

                    data.getFileDescriptor(), quotaBytes, transportFlags);



            long result = RESULT_ERROR;



            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();

            try {

                BackupAgent.this.onBackup(oldState, output, newState);

                result = RESULT_SUCCESS;
@@ -1111,9 +1112,6 @@ public abstract class BackupAgent extends ContextWrapper { 
        private void doRestoreInternal(ParcelFileDescriptor data, long appVersionCode,

                ParcelFileDescriptor newState, int token, IBackupManager callbackBinder,

                List<String> excludedKeys) throws RemoteException {

            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();



            if (DEBUG) Log.v(TAG, "doRestore() invoked");



            // Ensure that any side-effect SharedPreferences writes have landed *before*
@@ -1121,6 +1119,9 @@ public abstract class BackupAgent extends ContextWrapper { 
            waitForSharedPrefs();



            BackupDataInput input = new BackupDataInput(data.getFileDescriptor());



            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();

            try {

                BackupAgent.this.onRestore(input, appVersionCode, newState,

                        excludedKeys != null ? new HashSet<>(excludedKeys)
@@ -1152,15 +1153,14 @@ public abstract class BackupAgent extends ContextWrapper { 
        @Override

        public void doFullBackup(ParcelFileDescriptor data,

                long quotaBytes, int token, IBackupManager callbackBinder, int transportFlags) {

            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();



            if (DEBUG) Log.v(TAG, "doFullBackup() invoked");



            // Ensure that any SharedPreferences writes have landed *before*

            // we potentially try to back up the underlying files directly.

            waitForSharedPrefs();



            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();

            try {

                BackupAgent.this.onFullBackup(new FullBackupDataOutput(

                        data, quotaBytes, transportFlags));
@@ -1199,12 +1199,13 @@ public abstract class BackupAgent extends ContextWrapper { 


        public void doMeasureFullBackup(long quotaBytes, int token, IBackupManager callbackBinder,

                int transportFlags) {

            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();

            FullBackupDataOutput measureOutput =

                    new FullBackupDataOutput(quotaBytes, transportFlags);



            waitForSharedPrefs();



            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();

            try {

                BackupAgent.this.onFullBackup(measureOutput);

            } catch (IOException ex) {
@@ -1284,9 +1285,10 @@ public abstract class BackupAgent extends ContextWrapper { 
                long backupDataBytes,

                long quotaBytes,

                IBackupCallback callbackBinder) {

            final long ident = Binder.clearCallingIdentity();



            long result = RESULT_ERROR;



            // Ensure that we're running with the app's normal permission level

            final long ident = Binder.clearCallingIdentity();

            try {

                BackupAgent.this.onQuotaExceeded(backupDataBytes, quotaBytes);

                result = RESULT_SUCCESS;

core/java/android/bluetooth/BluetoothHidDevice.java

+42 −14
Original line number Diff line number Diff line
@@ -342,44 +342,72 @@ public final class BluetoothHidDevice implements BluetoothProfile { 


        @Override

        public void onAppStatusChanged(BluetoothDevice pluggedDevice, boolean registered) {

            clearCallingIdentity();

            final long token = clearCallingIdentity();

            try {

                mExecutor.execute(() -> mCallback.onAppStatusChanged(pluggedDevice, registered));

            } finally {

                restoreCallingIdentity(token);

            }

        }



        @Override

        public void onConnectionStateChanged(BluetoothDevice device, int state) {

            clearCallingIdentity();

            final long token = clearCallingIdentity();

            try {

                mExecutor.execute(() -> mCallback.onConnectionStateChanged(device, state));

            } finally {

                restoreCallingIdentity(token);

            }

        }



        @Override

        public void onGetReport(BluetoothDevice device, byte type, byte id, int bufferSize) {

            clearCallingIdentity();

            final long token = clearCallingIdentity();

            try {

                mExecutor.execute(() -> mCallback.onGetReport(device, type, id, bufferSize));

            } finally {

                restoreCallingIdentity(token);

            }

        }



        @Override

        public void onSetReport(BluetoothDevice device, byte type, byte id, byte[] data) {

            clearCallingIdentity();

            final long token = clearCallingIdentity();

            try {

                mExecutor.execute(() -> mCallback.onSetReport(device, type, id, data));

            } finally {

                restoreCallingIdentity(token);

            }

        }



        @Override

        public void onSetProtocol(BluetoothDevice device, byte protocol) {

            clearCallingIdentity();

            final long token = clearCallingIdentity();

            try {

                mExecutor.execute(() -> mCallback.onSetProtocol(device, protocol));

            } finally {

                restoreCallingIdentity(token);

            }

        }



        @Override

        public void onInterruptData(BluetoothDevice device, byte reportId, byte[] data) {

            clearCallingIdentity();

            final long token = clearCallingIdentity();

            try {

                mExecutor.execute(() -> mCallback.onInterruptData(device, reportId, data));

            } finally {

                restoreCallingIdentity(token);

            }

        }



        @Override

        public void onVirtualCableUnplug(BluetoothDevice device) {

            clearCallingIdentity();

            final long token = clearCallingIdentity();

            try {

                mExecutor.execute(() -> mCallback.onVirtualCableUnplug(device));

            } finally {

                restoreCallingIdentity(token);

            }

        }

    }

core/java/android/content/ContentProvider.java

+1 −0
Original line number Diff line number Diff line
@@ -1043,6 +1043,7 @@ public abstract class ContentProvider implements ContentInterface, ComponentCall 
     *         calling identity by passing it to

     *         {@link #restoreCallingIdentity}.

     */

    @SuppressWarnings("AndroidFrameworkBinderIdentity")

    public final @NonNull CallingIdentity clearCallingIdentity() {

        return new CallingIdentity(Binder.clearCallingIdentity(), setCallingPackage(null));

    }

core/java/android/hardware/camera2/impl/CameraDeviceImpl.java

+2 −1
Original line number Diff line number Diff line
@@ -1587,7 +1587,7 @@ public class CameraDeviceImpl extends CameraDevice 
            }



            switch (errorCode) {

                case CameraDeviceCallbacks.ERROR_CAMERA_DISCONNECTED:

                case CameraDeviceCallbacks.ERROR_CAMERA_DISCONNECTED: {

                    final long ident = Binder.clearCallingIdentity();

                    try {

                        mDeviceExecutor.execute(mCallOnDisconnected);
@@ -1595,6 +1595,7 @@ public class CameraDeviceImpl extends CameraDevice 
                        Binder.restoreCallingIdentity(ident);

                    }

                    break;

                }

                case CameraDeviceCallbacks.ERROR_CAMERA_REQUEST:

                case CameraDeviceCallbacks.ERROR_CAMERA_RESULT:

                case CameraDeviceCallbacks.ERROR_CAMERA_BUFFER:

core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java

+2 −1
Original line number Diff line number Diff line
@@ -147,7 +147,7 @@ public class CameraOfflineSessionImpl extends CameraOfflineSession 
                    case CameraDeviceCallbacks.ERROR_CAMERA_BUFFER:

                        onCaptureErrorLocked(errorCode, resultExtras);

                        break;

                    default:

                    default: {

                        Runnable errorDispatch = new Runnable() {

                            @Override

                            public void run() {
@@ -167,6 +167,7 @@ public class CameraOfflineSessionImpl extends CameraOfflineSession 
                    }

                }

            }

        }



        @Override

        public void onRepeatingRequestError(long lastFrameNumber, int repeatingRequestId) {