Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asb s master sep 2024 #2575

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ index 0daae6bdcb..d14bd65167 100644
# It must match one of the Android Security Patch Level strings of the Public Security Bulletins.
# If there is no $PLATFORM_SECURITY_PATCH set, keep it empty.
- PLATFORM_SECURITY_PATCH := 2022-06-05
+ PLATFORM_SECURITY_PATCH := 2024-08-01
+ PLATFORM_SECURITY_PATCH := 2024-09-01
endif
.KATI_READONLY := PLATFORM_SECURITY_PATCH

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
From 326c584ebafe625723ae5ddae597add1de4c1b33 Mon Sep 17 00:00:00 2001
From: Wonsik Kim <[email protected]>
Date: Fri, 28 Jun 2024 00:33:51 +0000
Subject: [PATCH] omx: check HDR10+ info param size

Bug: 329641908
Test: presubmit
Flag: EXEMPT security fix
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:53298956ba6bb8f147a632d7aaed8566dfc203ee)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f816148a719d2a3bbf432f11da98b3d5fa7de74f)
Merged-In: I72523e1de61e5f947174272b732e170e1c2964df
Change-Id: I72523e1de61e5f947174272b732e170e1c2964df
---
media/libstagefright/omx/SoftVideoDecoderOMXComponent.cpp | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/media/libstagefright/omx/SoftVideoDecoderOMXComponent.cpp b/media/libstagefright/omx/SoftVideoDecoderOMXComponent.cpp
index 418302389d..4ab5d10609 100644
--- a/media/libstagefright/omx/SoftVideoDecoderOMXComponent.cpp
+++ b/media/libstagefright/omx/SoftVideoDecoderOMXComponent.cpp
@@ -619,6 +619,13 @@ OMX_ERRORTYPE SoftVideoDecoderOMXComponent::getConfig(
if (!isValidOMXParam(outParams)) {
return OMX_ErrorBadParameter;
}
+ if (offsetof(DescribeHDR10PlusInfoParams, nValue) + outParams->nParamSize >
+ outParams->nSize) {
+ ALOGE("b/329641908: too large param size; nParamSize=%u nSize=%u",
+ outParams->nParamSize, outParams->nSize);
+ android_errorWriteLog(0x534e4554, "329641908");
+ return OMX_ErrorBadParameter;
+ }

outParams->nParamSizeUsed = info->size();

--
2.46.0.rc2.264.g509ed76dc8-goog

Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
From 8cb85afd554e0171a174d79aacc4e2200860cfb9 Mon Sep 17 00:00:00 2001
From: Kiran Ramachandra <[email protected]>
Date: Thu, 30 May 2024 21:21:12 +0000
Subject: [PATCH] DO NOT MERGE Ignore - Sanitized uri scheme by removing scheme
delimiter

Initially considered removing unsupported characters as per IANA guidelines, but this could break applications that use custom schemes with asterisks. Instead, opted to remove only the "://" to minimize disruption

Bug: 261721900
Test: atest FrameworksCoreTests:android.net.UriTest

No-Typo-Check: The unit test is specifically written to test few cases, string "http://https://" is not a typo

NOTE FOR REVIEWERS - original patch and result patch are not identical.
PLEASE REVIEW CAREFULLY.
Diffs between the patches:
@AsbSecurityTest(cveBugId = 261721900)
> + @SmallTest
> + public void testSchemeSanitization() {
> + Uri uri = new Uri.Builder()
> + .scheme("http://https://evil.com:/te:st/")
> + .authority("google.com").path("one/way").build();
> + assertEquals("httphttpsevil.com:/te:st/", uri.getScheme());
> + assertEquals("httphttpsevil.com:/te:st/://google.com/one/way", uri.toString());
> + }
> +

Original patch:
diff --git a/core/java/android/net/Uri.java b/core/java/android/net/Uri.java
old mode 100644
new mode 100644
---
core/java/android/net/Uri.java | 6 +++++-
core/tests/coretests/src/android/net/UriTest.java | 11 +++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/core/java/android/net/Uri.java b/core/java/android/net/Uri.java
index d71faee4cc8d..ed6705c8fa23 100644
--- a/core/java/android/net/Uri.java
+++ b/core/java/android/net/Uri.java
@@ -1391,7 +1391,11 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
* @param scheme name or {@code null} if this is a relative Uri
*/
public Builder scheme(String scheme) {
- this.scheme = scheme;
+ if (scheme != null) {
+ this.scheme = scheme.replace("://", "");
+ } else {
+ this.scheme = null;
+ }
return this;
}

diff --git a/core/tests/coretests/src/android/net/UriTest.java b/core/tests/coretests/src/android/net/UriTest.java
index 3733bfa586d1..35641285e3c5 100644
--- a/core/tests/coretests/src/android/net/UriTest.java
+++ b/core/tests/coretests/src/android/net/UriTest.java
@@ -18,6 +18,7 @@ package android.net;

import android.content.ContentUris;
import android.os.Parcel;
+import android.platform.test.annotations.AsbSecurityTest;

import androidx.test.filters.SmallTest;

@@ -88,6 +89,16 @@ public class UriTest extends TestCase {
assertNull(u.getHost());
}

+ @AsbSecurityTest(cveBugId = 261721900)
+ @SmallTest
+ public void testSchemeSanitization() {
+ Uri uri = new Uri.Builder()
+ .scheme("http://https://evil.com:/te:st/")
+ .authority("google.com").path("one/way").build();
+ assertEquals("httphttpsevil.com:/te:st/", uri.getScheme());
+ assertEquals("httphttpsevil.com:/te:st/://google.com/one/way", uri.toString());
+ }
+
@SmallTest
public void testStringUri() {
assertEquals("bob lee",
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
From b6f7fb547e85f729d9cd650b0544785e16b835de Mon Sep 17 00:00:00 2001
From: Nikolay Elenkov <[email protected]>
Date: Sun, 30 Jun 2024 06:20:30 +0000
Subject: [PATCH] RESTRICT AUTOMERGE Delete keystore keys from
RecoveryService.rebootRecoveryWithCommand()

Adds deleteSecrets() to RecoverySystemService. This method is called
from rebootRecoveryWithCommand () before the --wipe_data command is
passed to recovery and the device is force-rebooted.

deleteSecerts() calls IKeystoreMaintenance.deleteAllKeys() in order to
quickly destroy the keys protecting the synthetic password blobs
used to derive FBE encryption keys.

The intent is to make FBE-encrypted data unrecoverable even if the full
data wipe in recovery is interrupted or skipped.

Bug: 324321147
Test: Manual - System -> Reset options -> Erase all data.
Test: Hold VolDown key to interrupt reboot and stop at bootloader
screen.
Test: fastboot oem bcd wipe command && fastboot oem bcd wipe recovery
Test: fastboot reboot
Test: Device reboots into recovery and prompts to factory reset:
Test: 'Cannot load Android system. Your data may be corrupt. ...'
(cherry picked from https://android-review.googlesource.com/q/commit:0d00031851e9f5d8ef93947205a7e8b5257f0d8d)
Ignore-AOSP-First: Security fix backport
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:9cdf9eae2e02a6c3651379c33c4655368b009d13)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:1e81807b183f08c9b7a68d225afff8b9ffb60fbe)
Merged-In: I5eb8e97f3ae1a18d5e7e7c2c7eca048ebff3440a
Change-Id: I5eb8e97f3ae1a18d5e7e7c2c7eca048ebff3440a
---
.../security/AndroidKeyStoreMaintenance.java | 22 +++++++++++++++++++
.../recoverysystem/RecoverySystemService.java | 19 ++++++++++++++++
2 files changed, 41 insertions(+)

diff --git a/keystore/java/android/security/AndroidKeyStoreMaintenance.java b/keystore/java/android/security/AndroidKeyStoreMaintenance.java
index 919a93b8f107..b2d1755bb860 100644
--- a/keystore/java/android/security/AndroidKeyStoreMaintenance.java
+++ b/keystore/java/android/security/AndroidKeyStoreMaintenance.java
@@ -18,8 +18,10 @@ package android.security;

import android.annotation.NonNull;
import android.annotation.Nullable;
+import android.os.RemoteException;
import android.os.ServiceManager;
import android.os.ServiceSpecificException;
+import android.os.StrictMode;
import android.security.maintenance.IKeystoreMaintenance;
import android.system.keystore2.Domain;
import android.system.keystore2.KeyDescriptor;
@@ -183,4 +185,24 @@ public class AndroidKeyStoreMaintenance {
return SYSTEM_ERROR;
}
}
+
+ /**
+ * Deletes all keys in all KeyMint devices.
+ * Called by RecoverySystem before rebooting to recovery in order to delete all KeyMint keys,
+ * including synthetic password protector keys (used by LockSettingsService), as well as keys
+ * protecting DE and metadata encryption keys (used by vold). This ensures that FBE-encrypted
+ * data is unrecoverable even if the data wipe in recovery is interrupted or skipped.
+ */
+ public static void deleteAllKeys() throws KeyStoreException {
+ StrictMode.noteDiskWrite();
+ try {
+ getService().deleteAllKeys();
+ } catch (RemoteException | NullPointerException e) {
+ throw new KeyStoreException(SYSTEM_ERROR,
+ "Failure to connect to Keystore while trying to delete all keys.");
+ } catch (ServiceSpecificException e) {
+ throw new KeyStoreException(e.errorCode,
+ "Keystore error while trying to delete all keys.");
+ }
+ }
}
diff --git a/services/core/java/com/android/server/recoverysystem/RecoverySystemService.java b/services/core/java/com/android/server/recoverysystem/RecoverySystemService.java
index 13218731af70..23941bc338b8 100644
--- a/services/core/java/com/android/server/recoverysystem/RecoverySystemService.java
+++ b/services/core/java/com/android/server/recoverysystem/RecoverySystemService.java
@@ -52,6 +52,7 @@ import android.os.ShellCallback;
import android.os.SystemProperties;
import android.provider.DeviceConfig;
import android.sysprop.ApexProperties;
+import android.security.AndroidKeyStoreMaintenance;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.FastImmutableArraySet;
@@ -66,6 +67,7 @@ import com.android.internal.widget.RebootEscrowListener;
import com.android.server.LocalServices;
import com.android.server.SystemService;
import com.android.server.pm.ApexManager;
+import com.android.server.utils.Slogf;

import libcore.io.IoUtils;

@@ -117,6 +119,8 @@ public class RecoverySystemService extends IRecoverySystem.Stub implements Reboo
static final String LSKF_CAPTURED_TIMESTAMP_PREF = "lskf_captured_timestamp";
static final String LSKF_CAPTURED_COUNT_PREF = "lskf_captured_count";

+ static final String RECOVERY_WIPE_DATA_COMMAND = "--wipe_data";
+
private final Injector mInjector;
private final Context mContext;

@@ -511,17 +515,32 @@ public class RecoverySystemService extends IRecoverySystem.Stub implements Reboo
@Override // Binder call
public void rebootRecoveryWithCommand(String command) {
if (DEBUG) Slog.d(TAG, "rebootRecoveryWithCommand: [" + command + "]");
+
+ boolean isForcedWipe = command != null && command.contains(RECOVERY_WIPE_DATA_COMMAND);
synchronized (sRequestLock) {
if (!setupOrClearBcb(true, command)) {
return;
}

+ if (isForcedWipe) {
+ deleteSecrets();
+ }
+
// Having set up the BCB, go ahead and reboot.
PowerManager pm = mInjector.getPowerManager();
pm.reboot(PowerManager.REBOOT_RECOVERY);
}
}

+ private static void deleteSecrets() {
+ Slogf.w(TAG, "deleteSecrets");
+ try {
+ AndroidKeyStoreMaintenance.deleteAllKeys();
+ } catch (android.security.KeyStoreException e) {
+ Log.wtf(TAG, "Failed to delete all keys from keystore.", e);
+ }
+ }
+
private void enforcePermissionForResumeOnReboot() {
if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.RECOVERY)
!= PackageManager.PERMISSION_GRANTED
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
From a82c33e2e9e702214e932b25d27c25dcec448fc1 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <[email protected]>
Date: Mon, 6 May 2024 17:49:09 +0000
Subject: [PATCH] Fix permission bypasses to multiple methods

Researcher reports that some BT calls across Binder are validating only
BT's own permissions and not the calling app's permissions. On
investigation this seems to be due to a missing null check in several BT
permissions checks, which allows a malicious app to pass in a null
AttributionSource and therefore produce a stub AttributionSource chain
which does not properly check for the caller's permissions.

Add null checks.

Bug: 242996380
Test: atest UtilsTest
Test: researcher POC
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:244e4734d1ed316e8725b0f33e18d8eb709554f1)
Merged-In: I57d80cfa07bd6d3fd410a01764b3db2db9b41b81
Change-Id: I57d80cfa07bd6d3fd410a01764b3db2db9b41b81
---
src/com/android/bluetooth/Utils.java | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/com/android/bluetooth/Utils.java b/src/com/android/bluetooth/Utils.java
index f1e8e0f10..ab30c3e55 100644
--- a/src/com/android/bluetooth/Utils.java
+++ b/src/com/android/bluetooth/Utils.java
@@ -79,6 +79,7 @@ import java.nio.charset.CharsetDecoder;
import java.time.Instant;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
+import java.util.Objects;
import java.util.UUID;
import java.util.concurrent.TimeUnit;

@@ -451,7 +452,8 @@ public final class Utils {
// attributionSource.enforceCallingUid();
final int result = PermissionChecker.checkPermissionForDataDeliveryFromDataSource(
context, permission, PID_UNKNOWN,
- new AttributionSource(context.getAttributionSource(), attributionSource), message);
+ new AttributionSource(context.getAttributionSource(),
+ Objects.requireNonNull(attributionSource)), message);
if (result == PERMISSION_GRANTED) {
return true;
}
@@ -693,7 +695,8 @@ public final class Utils {
// attributionSource.enforceCallingUid();
if (PermissionChecker.checkPermissionForDataDeliveryFromDataSource(
context, ACCESS_COARSE_LOCATION, PID_UNKNOWN,
- new AttributionSource(context.getAttributionSource(), attributionSource),
+ new AttributionSource(context.getAttributionSource(),
+ Objects.requireNonNull(attributionSource)),
"Bluetooth location check") == PERMISSION_GRANTED) {
return true;
}
@@ -721,14 +724,16 @@ public final class Utils {
// attributionSource.enforceCallingUid();
if (PermissionChecker.checkPermissionForDataDeliveryFromDataSource(
context, ACCESS_FINE_LOCATION, PID_UNKNOWN,
- new AttributionSource(context.getAttributionSource(), attributionSource),
+ new AttributionSource(context.getAttributionSource(),
+ Objects.requireNonNull(attributionSource)),
"Bluetooth location check") == PERMISSION_GRANTED) {
return true;
}

if (PermissionChecker.checkPermissionForDataDeliveryFromDataSource(
context, ACCESS_COARSE_LOCATION, PID_UNKNOWN,
- new AttributionSource(context.getAttributionSource(), attributionSource),
+ new AttributionSource(context.getAttributionSource(),
+ Objects.requireNonNull(attributionSource)),
"Bluetooth location check") == PERMISSION_GRANTED) {
return true;
}
@@ -755,7 +760,8 @@ public final class Utils {
// attributionSource.enforceCallingUid();
if (PermissionChecker.checkPermissionForDataDeliveryFromDataSource(
context, ACCESS_FINE_LOCATION, PID_UNKNOWN,
- new AttributionSource(context.getAttributionSource(), attributionSource),
+ new AttributionSource(context.getAttributionSource(),
+ Objects.requireNonNull(attributionSource)),
"Bluetooth location check") == PERMISSION_GRANTED) {
return true;
}
--
2.46.0.rc2.264.g509ed76dc8-goog

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
From ccabd3d0e7e921941d54180970d5e7de260d32e9 Mon Sep 17 00:00:00 2001
From: Chaohui Wang <[email protected]>
Date: Thu, 2 Nov 2023 11:43:00 +0800
Subject: [PATCH] Limit wifi item edit content's max length to 500

Bug: 293199910
Test: manual - on "Add network"

(cherry picked from commit 855053ca4124f2d515b21c469096f8c18bd4829d)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:092668676af741719d50ac0f121a8f8461aa21ad)
Merged-In: I303b8c6e0f3c3a1174a047ba98f302042e5db9ae
Change-Id: I303b8c6e0f3c3a1174a047ba98f302042e5db9ae
---
res/values/styles.xml | 1 +
1 file changed, 1 insertion(+)

diff --git a/res/values/styles.xml b/res/values/styles.xml
index 8402dec73c..9a9477bb5d 100644
--- a/res/values/styles.xml
+++ b/res/values/styles.xml
@@ -148,6 +148,7 @@
<item name="android:textAppearance">@android:style/TextAppearance.DeviceDefault.Medium</item>
<item name="android:textColorHint">?android:attr/textColorSecondary</item>
<item name="android:minHeight">@dimen/min_tap_target_size</item>
+ <item name="android:maxLength">500</item>
</style>

<style name="wifi_section">
--
2.46.0.rc2.264.g509ed76dc8-goog

Loading