ASB cherrypicks

https://review.lineageos.org/q/topic:%22n-asb-2023-09%22

Signed-off-by: Tad <tad@spotco.us>
This commit is contained in:
Tad 2023-09-05 20:50:31 -04:00
parent 0ec3c25d86
commit 3e8effa345
No known key found for this signature in database
GPG Key ID: B286E9F57A07424B
7 changed files with 416 additions and 0 deletions

View File

@ -0,0 +1,32 @@
From ef4c2b8495ff729d8f70d5f1cbceed6a36ff94a1 Mon Sep 17 00:00:00 2001
From: Shruti Bihani <shrutibihani@google.com>
Date: Thu, 6 Jul 2023 08:41:56 +0000
Subject: [PATCH] Fix Segv on unknown address error flagged by fuzzer test.
The error is thrown when the destructor tries to free pointer memory.
This is happening for cases where the pointer was not initialized. Initializing it to a default value fixes the error.
Bug: 245135112
Test: Build mtp_host_property_fuzzer and run on the target device
(cherry picked from commit 3afa6e80e8568fe63f893fa354bc79ef91d3dcc0)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:99d0823ca2b8275f000a437150fb8d1938b1b31a)
Merged-In: I255cd68b7641e96ac47ab81479b9b46b78c15580
Change-Id: I255cd68b7641e96ac47ab81479b9b46b78c15580
---
media/mtp/MtpProperty.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/media/mtp/MtpProperty.h b/media/mtp/MtpProperty.h
index 03c08e1eed..45fcbe8917 100644
--- a/media/mtp/MtpProperty.h
+++ b/media/mtp/MtpProperty.h
@@ -24,6 +24,9 @@ namespace android {
class MtpDataPacket;
struct MtpPropertyValue {
+ // pointer str initialized to NULL so that free operation
+ // is not called for pre-assigned value
+ MtpPropertyValue() : str (NULL) {}
union {
int8_t i8;
uint8_t u8;

View File

@ -0,0 +1,137 @@
From 4ee14083484520e5b8e38573e1da50e2b496167a Mon Sep 17 00:00:00 2001
From: Ashish Kumar <akgaurav@google.com>
Date: Fri, 26 May 2023 14:18:46 +0000
Subject: [PATCH] Fixed leak of cross user data in multiple settings.
- Any app is allowed to receive GET_CONTENT intent. Using this, an user puts back in the intent an uri with data of another user.
- Telephony service has INTERACT_ACROSS_USER permission. Using this, it reads and shows the deta to the evil user.
Fix: When telephony service gets the intent result, it checks if the uri is from the current user or not.
Bug: b/256591023 , b/256819787
Test: The malicious behaviour was not being reproduced. Unable to import contact from other users data.
Test2: Able to import contact from the primary user or uri with no user id
(These settings are not available for secondary users)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ab593467e900d4a6d25a34024a06195ae863f6dc)
Merged-In: I1e3a643f17948153aecc1d0df9ffd9619ad678c1
Change-Id: I1e3a643f17948153aecc1d0df9ffd9619ad678c1
Change-Id: I01cf83cc40bbf13497bc6c90f949f5bb62a833d7
---
.../android/phone/GsmUmtsCallForwardOptions.java | 12 ++++++++++++
.../phone/settings/VoicemailSettingsActivity.java | 13 +++++++++++++
.../phone/settings/fdn/EditFdnContactScreen.java | 11 +++++++++++
3 files changed, 36 insertions(+)
diff --git a/src/com/android/phone/GsmUmtsCallForwardOptions.java b/src/com/android/phone/GsmUmtsCallForwardOptions.java
index 99f3599742..f5e7ad295e 100644
--- a/src/com/android/phone/GsmUmtsCallForwardOptions.java
+++ b/src/com/android/phone/GsmUmtsCallForwardOptions.java
@@ -8,6 +8,7 @@
import android.app.AlertDialog;
import android.app.Dialog;
import android.content.Context;
+import android.content.ContentProvider;
import android.content.DialogInterface;
import android.content.Intent;
import android.database.Cursor;
@@ -15,6 +16,8 @@
import android.net.NetworkInfo;
import android.os.Bundle;
import android.os.PersistableBundle;
+import android.os.Process;
+import android.os.UserHandle;
import android.preference.Preference;
import android.preference.PreferenceScreen;
import android.telephony.ServiceState;
@@ -229,6 +232,15 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
}
Cursor cursor = null;
try {
+ // check if the URI returned by the user belongs to the user
+ final int currentUser = UserHandle.getUserId(Process.myUid());
+ if (currentUser
+ != ContentProvider.getUserIdFromUri(data.getData(), currentUser)) {
+
+ Log.w(LOG_TAG, "onActivityResult: Contact data of different user, "
+ + "cannot access");
+ return;
+ }
cursor = getContentResolver().query(data.getData(),
NUM_PROJECTION, null, null, null);
if ((cursor == null) || (!cursor.moveToFirst())) {
diff --git a/src/com/android/phone/settings/VoicemailSettingsActivity.java b/src/com/android/phone/settings/VoicemailSettingsActivity.java
index fea702bf53..1a4b1eea45 100644
--- a/src/com/android/phone/settings/VoicemailSettingsActivity.java
+++ b/src/com/android/phone/settings/VoicemailSettingsActivity.java
@@ -17,6 +17,7 @@
package com.android.phone.settings;
import android.app.Dialog;
+import android.content.ContentProvider;
import android.content.DialogInterface;
import android.content.Intent;
import android.database.Cursor;
@@ -24,6 +25,7 @@
import android.os.Bundle;
import android.os.Handler;
import android.os.Message;
+import android.os.Process;
import android.os.UserHandle;
import android.preference.CheckBoxPreference;
import android.preference.Preference;
@@ -544,6 +546,17 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
Cursor cursor = null;
try {
+ // check if the URI returned by the user belongs to the user
+ final int currentUser = UserHandle.getUserId(Process.myUid());
+ if (currentUser
+ != ContentProvider.getUserIdFromUri(data.getData(), currentUser)) {
+
+ if (DBG) {
+ log("onActivityResult: Contact data of different user, "
+ + "cannot access");
+ }
+ return;
+ }
cursor = getContentResolver().query(data.getData(),
new String[] { CommonDataKinds.Phone.NUMBER }, null, null, null);
if ((cursor == null) || (!cursor.moveToFirst())) {
diff --git a/src/com/android/phone/settings/fdn/EditFdnContactScreen.java b/src/com/android/phone/settings/fdn/EditFdnContactScreen.java
index 23ef0bb075..98dbd9480c 100644
--- a/src/com/android/phone/settings/fdn/EditFdnContactScreen.java
+++ b/src/com/android/phone/settings/fdn/EditFdnContactScreen.java
@@ -21,6 +21,7 @@
import android.app.Activity;
import android.content.AsyncQueryHandler;
+import android.content.ContentProvider;
import android.content.ContentResolver;
import android.content.ContentValues;
import android.content.Intent;
@@ -29,6 +30,8 @@
import android.net.Uri;
import android.os.Bundle;
import android.os.Handler;
+import android.os.Process;
+import android.os.UserHandle;
import android.provider.Contacts.PeopleColumns;
import android.provider.Contacts.PhonesColumns;
import android.provider.ContactsContract.CommonDataKinds;
@@ -152,6 +155,14 @@ protected void onActivityResult(int requestCode, int resultCode, Intent intent)
}
Cursor cursor = null;
try {
+ // check if the URI returned by the user belongs to the user
+ final int currentUser = UserHandle.getUserId(Process.myUid());
+ if (currentUser
+ != ContentProvider.getUserIdFromUri(intent.getData(), currentUser)) {
+ Log.w(LOG_TAG, "onActivityResult: Contact data of different user, "
+ + "cannot access");
+ return;
+ }
cursor = getContentResolver().query(intent.getData(),
NUM_PROJECTION, null, null, null);
if ((cursor == null) || (!cursor.moveToFirst())) {

View File

@ -0,0 +1,71 @@
From 2ccddcaa7525966043121fcbe6b806246fdec327 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Tue, 18 Apr 2023 23:58:50 +0000
Subject: [PATCH] Fix integer overflow in build_read_multi_rsp
Local variables tracking structure size in build_read_multi_rsp are of
uint16 type but accept a full uint16 range from function arguments while
appending a fixed-length offset. This can lead to an integer overflow
and unexpected behavior.
Change the locals to size_t, and add a check during reasssignment.
Bug: 273966636
Test: atest bluetooth_test_gd_unit, net_test_stack_btm
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from commit 70a4d628fa016a9487fae07f211644b95e1f0000)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:badb8ffce06b517cbcfdbfa68cb7b7e02d22494a)
Merged-In: I3a74bdb0d003cb6bf4f282615be8c68836676715
Change-Id: I3a74bdb0d003cb6bf4f282615be8c68836676715
---
stack/gatt/gatt_sr.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/stack/gatt/gatt_sr.c b/stack/gatt/gatt_sr.c
index c2cdb885dcf..6457a37589f 100644
--- a/stack/gatt/gatt_sr.c
+++ b/stack/gatt/gatt_sr.c
@@ -122,7 +122,8 @@ void gatt_dequeue_sr_cmd (tGATT_TCB *p_tcb)
static BOOLEAN process_read_multi_rsp (tGATT_SR_CMD *p_cmd, tGATT_STATUS status,
tGATTS_RSP *p_msg, UINT16 mtu)
{
- UINT16 ii, total_len, len;
+ UINT16 ii;
+ size_t total_len, len;
UINT8 *p;
BOOLEAN is_overflow = FALSE;
@@ -182,7 +183,7 @@ static BOOLEAN process_read_multi_rsp (tGATT_SR_CMD *p_cmd, tGATT_STATUS status,
/* just send the partial response for the overflow case */
len = p_rsp->attr_value.len - (total_len - mtu);
is_overflow = TRUE;
- GATT_TRACE_DEBUG ("multi read overflow available len=%d val_len=%d", len, p_rsp->attr_value.len );
+ GATT_TRACE_DEBUG ("multi read overflow available len=%zu val_len=%d", len, p_rsp->attr_value.len );
}
else
{
@@ -191,10 +192,19 @@ static BOOLEAN process_read_multi_rsp (tGATT_SR_CMD *p_cmd, tGATT_STATUS status,
if (p_rsp->attr_value.handle == p_cmd->multi_req.handles[ii])
{
- memcpy (p, p_rsp->attr_value.value, len);
- if (!is_overflow)
- p += len;
- p_buf->len += len;
+ // check for possible integer overflow
+ if (p_buf->len + len <= UINT16_MAX)
+ {
+ memcpy(p, p_rsp->attr_value.value, len);
+ if (!is_overflow)
+ p += len;
+ p_buf->len += len;
+ }
+ else
+ {
+ p_cmd->status = GATT_NOT_FOUND;
+ break;
+ }
}
else
{

View File

@ -0,0 +1,88 @@
From 6639c1e4ff7b39f7b9b7e5d924ff4c36cab556eb Mon Sep 17 00:00:00 2001
From: Qiyu Hu <qiyuh@google.com>
Date: Wed, 13 Jun 2018 08:08:17 -0700
Subject: [PATCH] Fix reliable write.
We cannot simply assume the write is terminated in reliable write. When
the reliable write value is longer than MTU allows, the current
implementation can only send whatever MTU allows and naively set the
status to GATT_SUCCESS, in the name of "application should verify handle
offset and value are matched or not". That's why MTU negotiation is a
workaround as people mention in b/37031096, which just fits all the write
value into a single request.
This also blocks our test on CtsVerifier.
Bug: 37031096
Test: Manual test and confirm that we don't simply send partial value
Change-Id: I907877608f4672f24c002e630e58bf9133937a5e
---
stack/gatt/gatt_cl.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/stack/gatt/gatt_cl.c b/stack/gatt/gatt_cl.c
index 04a027fef55..1033b9324f0 100644
--- a/stack/gatt/gatt_cl.c
+++ b/stack/gatt/gatt_cl.c
@@ -321,7 +321,7 @@ void gatt_send_queue_write_cancel (tGATT_TCB *p_tcb, tGATT_CLCB *p_clcb, tGATT_E
BOOLEAN gatt_check_write_long_terminate(tGATT_TCB *p_tcb, tGATT_CLCB *p_clcb, tGATT_VALUE *p_rsp_value)
{
tGATT_VALUE *p_attr = (tGATT_VALUE *)p_clcb->p_attr_buf;
- BOOLEAN exec = FALSE;
+ BOOLEAN terminate = FALSE;
tGATT_EXEC_FLAG flag = GATT_PREP_WRITE_EXEC;
GATT_TRACE_DEBUG("gatt_check_write_long_terminate ");
@@ -335,22 +335,21 @@ BOOLEAN gatt_check_write_long_terminate(tGATT_TCB *p_tcb, tGATT_CLCB *p_clcb, t
/* data does not match */
p_clcb->status = GATT_ERROR;
flag = GATT_PREP_WRITE_CANCEL;
- exec = TRUE;
+ terminate = TRUE;
}
else /* response checking is good */
{
p_clcb->status = GATT_SUCCESS;
/* update write offset and check if end of attribute value */
if ((p_attr->offset += p_rsp_value->len) >= p_attr->len)
- exec = TRUE;
+ terminate = TRUE;
}
}
- if (exec)
+ if (terminate && p_clcb->op_subtype != GATT_WRITE_PREPARE)
{
gatt_send_queue_write_cancel (p_tcb, p_clcb, flag);
- return TRUE;
}
- return FALSE;
+ return terminate;
}
/*******************************************************************************
**
@@ -653,19 +652,18 @@ void gatt_process_prep_write_rsp (tGATT_TCB *p_tcb, tGATT_CLCB *p_clcb, UINT8 op
memcpy (value.value, p, value.len);
+ if (!gatt_check_write_long_terminate(p_tcb, p_clcb, &value))
+ {
+ gatt_send_prepare_write(p_tcb, p_clcb);
+ return;
+ }
+
if (p_clcb->op_subtype == GATT_WRITE_PREPARE)
{
- p_clcb->status = GATT_SUCCESS;
/* application should verify handle offset
and value are matched or not */
-
gatt_end_operation(p_clcb, p_clcb->status, &value);
}
- else if (p_clcb->op_subtype == GATT_WRITE )
- {
- if (!gatt_check_write_long_terminate(p_tcb, p_clcb, &value))
- gatt_send_prepare_write(p_tcb, p_clcb);
- }
}
/*******************************************************************************

View File

@ -0,0 +1,45 @@
From 8e5178ea9e60fc2bbfee12c37bc61d0730327161 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Tue, 11 Apr 2023 23:05:45 +0000
Subject: [PATCH] Fix UAF in gatt_cl.cc
gatt_cl.cc accesses a header field after the buffer holding it may have
been freed.
Track the relevant state as a local variable instead.
Bug: 274617156
Test: atest: bluetooth, validated against fuzzer
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:cbaa83627b328eee8f2e26188909a5ebfb0388d5)
Merged-In: I085ecfa1a9ba098ecbfecbd3cb3e263ae13f9724
Change-Id: I085ecfa1a9ba098ecbfecbd3cb3e263ae13f9724
---
stack/gatt/gatt_cl.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/stack/gatt/gatt_cl.c b/stack/gatt/gatt_cl.c
index 1033b9324f..c78963f9ab 100644
--- a/stack/gatt/gatt_cl.c
+++ b/stack/gatt/gatt_cl.c
@@ -652,13 +652,18 @@ void gatt_process_prep_write_rsp (tGATT_TCB *p_tcb, tGATT_CLCB *p_clcb, UINT8 op
memcpy (value.value, p, value.len);
+ BOOLEAN subtype_is_write_prepare = (p_clcb->op_subtype == GATT_WRITE_PREPARE);
+
if (!gatt_check_write_long_terminate(p_tcb, p_clcb, &value))
{
gatt_send_prepare_write(p_tcb, p_clcb);
return;
}
- if (p_clcb->op_subtype == GATT_WRITE_PREPARE)
+ // We now know that we have not terminated, or else we would have returned
+ // early. We free the buffer only if the subtype is not equal to
+ // GATT_WRITE_PREPARE, so checking here is adequate to prevent UAF.
+ if (subtype_is_write_prepare)
{
/* application should verify handle offset
and value are matched or not */

View File

@ -0,0 +1,37 @@
From 49c2659fc32c183b135d49a88b93a7ebd1f664ed Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Wed, 10 May 2023 23:34:20 +0000
Subject: [PATCH] Fix an integer overflow bug in avdt_msg_asmbl
Bug: 280633699
Test: manual
Ignore-AOSP-First: security
Tag: #security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:bf9449a704c2983861dbe0ede9ab660e42826179)
Merged-In: Iaa4d603921fc4ffb8cfb5783f99ec0963affd6a2
Change-Id: Iaa4d603921fc4ffb8cfb5783f99ec0963affd6a2
---
stack/avdt/avdt_msg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/stack/avdt/avdt_msg.c b/stack/avdt/avdt_msg.c
index 91a58403e94..83902849b1e 100644
--- a/stack/avdt/avdt_msg.c
+++ b/stack/avdt/avdt_msg.c
@@ -1448,14 +1448,14 @@ BT_HDR *avdt_msg_asmbl(tAVDT_CCB *p_ccb, BT_HDR *p_buf)
* NOTE: The buffer is allocated above at the beginning of the
* reassembly, and is always of size BT_DEFAULT_BUFFER_SIZE.
*/
- UINT16 buf_len = BT_DEFAULT_BUFFER_SIZE - sizeof(BT_HDR);
+ size_t buf_len = BT_DEFAULT_BUFFER_SIZE - sizeof(BT_HDR);
/* adjust offset and len of fragment for header byte */
p_buf->offset += AVDT_LEN_TYPE_CONT;
p_buf->len -= AVDT_LEN_TYPE_CONT;
/* verify length */
- if ((p_ccb->p_rx_msg->offset + p_buf->len) > buf_len) {
+ if (((size_t) p_ccb->p_rx_msg->offset + (size_t) p_buf->len) > buf_len) {
/* won't fit; free everything */
AVDT_TRACE_WARNING("%s: Fragmented message too big!", __func__);
osi_free_and_reset((void **)&p_ccb->p_rx_msg);

View File

@ -156,6 +156,7 @@ applyPatch "$DOS_PATCHES/android_frameworks_av/212799.patch"; #FLAC extractor CV
applyPatch "$DOS_PATCHES/android_frameworks_av/319987.patch"; #n-asb-2021-12 Fix heap-buffer-overflow in MPEG4Extractor
applyPatch "$DOS_PATCHES/android_frameworks_av/321222.patch"; #n-asb-2022-01 SimpleDecodingSource:Prevent OOB write in heap mem
applyPatch "$DOS_PATCHES/android_frameworks_av/358729.patch"; #n-asb-2023-06 Fix NuMediaExtractor::readSampleData buffer Handling
applyPatch "$DOS_PATCHES/android_frameworks_av/365698.patch"; #n-asb-2023-09 Fix Segv on unknown address error flagged by fuzzer test.
fi;
if enterAndClear "frameworks/base"; then
@ -438,6 +439,7 @@ fi;
if enterAndClear "packages/services/Telephony"; then
applyPatch "$DOS_PATCHES/android_packages_services_Telephony/346954.patch"; #n-asb-2023-01 Prevent overlays on the phone settings
applyPatch "$DOS_PATCHES/android_packages_services_Telephony/365699.patch"; #n-asb-2023-09 Fixed leak of cross user data in multiple settings.
applyPatch "$DOS_PATCHES/android_packages_services_Telephony/0001-PREREQ_Handle_All_Modes.patch"; #(DivestOS)
applyPatch "$DOS_PATCHES/android_packages_services_Telephony/0002-More_Preferred_Network_Modes.patch";
fi;
@ -488,6 +490,10 @@ applyPatch "$DOS_PATCHES/android_system_bt/358735.patch"; #n-asb-2023-06 Prevent
applyPatch "$DOS_PATCHES/android_system_bt/358736.patch"; #n-asb-2023-06 Revert "Revert "[RESTRICT AUTOMERGE] Validate buffer length in sdpu_build_uuid_seq""
applyPatch "$DOS_PATCHES/android_system_bt/358737.patch"; #n-asb-2023-06 Revert "Revert "Fix wrong BR/EDR link key downgrades (P_256->P_192)""
applyPatch "$DOS_PATCHES/android_system_bt/360892.patch"; #n-asb-2023-07 Fix gatt_end_operation buffer overflow
applyPatch "$DOS_PATCHES/android_system_bt/365694.patch"; #n-asb-2023-09 Fix integer overflow in build_read_multi_rsp
applyPatch "$DOS_PATCHES/android_system_bt/365695.patch"; #n-asb-2023-09 Fix reliable write.
applyPatch "$DOS_PATCHES/android_system_bt/365696.patch"; #n-asb-2023-09 Fix UAF in gatt_cl.cc
applyPatch "$DOS_PATCHES/android_system_bt/365697.patch"; #n-asb-2023-09 Fix an integer overflow bug in avdt_msg_asmbl
applyPatch "$DOS_PATCHES/android_system_bt/229574.patch"; #bt-sbc-hd-dualchannel-nougat: Increase maximum Bluetooth SBC codec bitrate for SBC HD (ValdikSS)
applyPatch "$DOS_PATCHES/android_system_bt/229575.patch"; #bt-sbc-hd-dualchannel-nougat: Explicit SBC Dual Channel (SBC HD) support (ValdikSS)
applyPatch "$DOS_PATCHES/android_system_bt/242134.patch"; #avrc_bld_get_attrs_rsp - fix attribute length position off by one (cprhokie)