Make vault lock intent used in notifications more explicit

Aegis can display a notification to the user as a reminder that the vault is
unlocked. If the user taps the notification, the vault is locked. CodeQL
reported that Aegis may be vulnerable to CWE-927, because of the use of an
implicit intent wrapped by a PendingIntent in that notification.

This does not appear to be exploitable in our case, because we use
``PendingIntent.getBroadcast`` and explicitly set the action of the wrapped
intent. Aegis also does not read or act on any information from the received
intent. This means that a malicious app cannot launch activities or send a
broadcast with a different action, as is common with these type of weakness. The
worst an app with notification access can do, is lock the vault.

Either way, it's good to make the intent explicit, so this patch addresses that.
Additionally, for API level 23 and up, we've made the wrapped intent immutable a
while back.

We'd like to thank John Rune, who ran a CodeQL scan on the Aegis codebase and
privately disclosed this finding to us.
This commit is contained in:
Alexander Bakker 2022-10-09 11:27:20 +02:00
parent 8c9ab38153
commit 3927ddec3e
4 changed files with 56 additions and 30 deletions

View file

@ -93,6 +93,13 @@
<service android:name=".services.NotificationService" />
<receiver android:name=".receivers.VaultLockReceiver" android:exported="false">
<intent-filter>
<action android:name="${applicationId}.LOCK_VAULT" />
<action android:name="android.intent.action.SCREEN_OFF" />
</intent-filter>
</receiver>
<provider
android:name="androidx.core.content.FileProvider"
android:authorities="${fileProviderAuthority}"

View file

@ -3,10 +3,7 @@ package com.beemdevelopment.aegis;
import android.app.Application;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.content.pm.ShortcutInfo;
import android.content.pm.ShortcutManager;
import android.graphics.drawable.Icon;
@ -35,7 +32,6 @@ import dagger.hilt.components.SingletonComponent;
public abstract class AegisApplicationBase extends Application {
private static final String CODE_LOCK_STATUS_ID = "lock_status_channel";
private static final String CODE_LOCK_VAULT_ACTION = "lock_vault";
private VaultManager _vaultManager;
@ -52,13 +48,6 @@ public abstract class AegisApplicationBase extends Application {
Iconics.init(this);
Iconics.registerFont(new MaterialDesignIconic());
// listen for SCREEN_OFF events
ScreenOffReceiver receiver = new ScreenOffReceiver();
IntentFilter intentFilter = new IntentFilter();
intentFilter.addAction(Intent.ACTION_SCREEN_OFF);
intentFilter.addAction(CODE_LOCK_VAULT_ACTION);
registerReceiver(receiver, intentFilter);
// lock the app if the user moves the application to the background
ProcessLifecycleOwner.get().getLifecycle().addObserver(new AppLifecycleObserver());
@ -121,15 +110,6 @@ public abstract class AegisApplicationBase extends Application {
}
}
private class ScreenOffReceiver extends BroadcastReceiver {
@Override
public void onReceive(Context context, Intent intent) {
if (_vaultManager.isAutoLockEnabled(Preferences.AUTO_LOCK_ON_DEVICE_LOCK)) {
_vaultManager.lock(false);
}
}
}
@EarlyEntryPoint
@InstallIn(SingletonComponent.class)
interface EntryPoint {

View file

@ -0,0 +1,34 @@
package com.beemdevelopment.aegis.receivers;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import com.beemdevelopment.aegis.BuildConfig;
import com.beemdevelopment.aegis.Preferences;
import com.beemdevelopment.aegis.vault.VaultManager;
import javax.inject.Inject;
import dagger.hilt.android.AndroidEntryPoint;
@AndroidEntryPoint
public class VaultLockReceiver extends BroadcastReceiver {
public static final String ACTION_LOCK_VAULT
= String.format("%s.LOCK_VAULT", BuildConfig.APPLICATION_ID);
@Inject
protected VaultManager _vaultManager;
@Override
public void onReceive(Context context, Intent intent) {
if (!intent.getAction().equals(ACTION_LOCK_VAULT)
&& !intent.getAction().equals(Intent.ACTION_SCREEN_OFF)) {
return;
}
if (_vaultManager.isAutoLockEnabled(Preferences.AUTO_LOCK_ON_DEVICE_LOCK)) {
_vaultManager.lock(false);
}
}
}

View file

@ -11,13 +11,14 @@ import androidx.annotation.Nullable;
import androidx.core.app.NotificationCompat;
import androidx.core.app.NotificationManagerCompat;
import com.beemdevelopment.aegis.BuildConfig;
import com.beemdevelopment.aegis.R;
import com.beemdevelopment.aegis.receivers.VaultLockReceiver;
public class NotificationService extends Service {
public static final int VAULT_UNLOCKED_ID = 1;
private static final int NOTIFICATION_VAULT_UNLOCKED = 1;
private static final String CODE_LOCK_STATUS_ID = "lock_status_channel";
private static final String CODE_LOCK_VAULT_ACTION = "lock_vault";
private static final String CHANNEL_ID = "lock_status_channel";
@Override
public int onStartCommand(Intent intent,int flags, int startId){
@ -28,28 +29,32 @@ public class NotificationService extends Service {
@SuppressLint("LaunchActivityFromNotification")
public void serviceMethod() {
int flags = 0;
Intent intent = new Intent(this, VaultLockReceiver.class);
intent.setAction(VaultLockReceiver.ACTION_LOCK_VAULT);
intent.setPackage(BuildConfig.APPLICATION_ID);
int flags = PendingIntent.FLAG_ONE_SHOT;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
flags |= PendingIntent.FLAG_IMMUTABLE;
}
Intent intentAction = new Intent(CODE_LOCK_VAULT_ACTION);
PendingIntent lockDatabaseIntent = PendingIntent.getBroadcast(this, 1, intentAction, flags);
NotificationCompat.Builder builder = new NotificationCompat.Builder(this, CODE_LOCK_STATUS_ID)
PendingIntent pendingIntent = PendingIntent.getBroadcast(this, 1, intent, flags);
NotificationCompat.Builder builder = new NotificationCompat.Builder(this, CHANNEL_ID)
.setSmallIcon(R.drawable.ic_aegis_notification)
.setContentTitle(getString(R.string.app_name_full))
.setContentText(getString(R.string.vault_unlocked_state))
.setPriority(NotificationCompat.PRIORITY_DEFAULT)
.setOngoing(true)
.setContentIntent(lockDatabaseIntent);
.setContentIntent(pendingIntent);
NotificationManagerCompat notificationManager = NotificationManagerCompat.from(this);
notificationManager.notify(VAULT_UNLOCKED_ID, builder.build());
notificationManager.notify(NOTIFICATION_VAULT_UNLOCKED, builder.build());
}
@Override
public void onDestroy() {
NotificationManagerCompat notificationManager = NotificationManagerCompat.from(this);
notificationManager.cancel(VAULT_UNLOCKED_ID);
notificationManager.cancel(NOTIFICATION_VAULT_UNLOCKED);
super.onDestroy();
}