Skip to content
This repository was archived by the owner on Dec 23, 2024. It is now read-only.

Commit d7996e8

Browse files
14.1: December ASB picks
Signed-off-by: Tad <[email protected]>
1 parent ee3e067 commit d7996e8

File tree

13 files changed

+774
-1
lines changed

13 files changed

+774
-1
lines changed
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
From 0d5107c9fa5780919b18cd15c1a4a073ac6ed7cd Mon Sep 17 00:00:00 2001
2+
From: Kweku Adams <[email protected]>
3+
Date: Fri, 23 Sep 2022 21:06:53 +0000
4+
Subject: [PATCH] RESTRICT AUTOMERGE: Drop invalid data.
5+
6+
Drop invalid data when writing or reading from XML. PersistableBundle
7+
does lazy unparcelling, so checking the values during unparcelling would
8+
remove the benefit of the lazy unparcelling. Checking the validity when
9+
writing to or reading from XML seems like the best alternative.
10+
11+
Bug: 246542285
12+
Bug: 247513680
13+
Test: install test app with invalid job config, start app to schedule job, then check logcat and jobscheduler persisted file
14+
(cherry picked from commit 666e8ac60a31e2cc52b335b41004263f28a8db06)
15+
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:62b37ab21ce27746a79a2071deee98c61b23c8d9)
16+
Merged-In: Ie817aa0993e9046cb313a750d2323cadc8c1ef15
17+
Change-Id: Ie817aa0993e9046cb313a750d2323cadc8c1ef15
18+
---
19+
core/java/android/os/PersistableBundle.java | 42 +++++++++++++++++----
20+
1 file changed, 34 insertions(+), 8 deletions(-)
21+
22+
diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java
23+
index db9f4b14a345e..c3e4b759b103c 100644
24+
--- a/core/java/android/os/PersistableBundle.java
25+
+++ b/core/java/android/os/PersistableBundle.java
26+
@@ -18,6 +18,7 @@
27+
28+
import android.annotation.Nullable;
29+
import android.util.ArrayMap;
30+
+import android.util.Slog;
31+
32+
import com.android.internal.util.XmlUtils;
33+
34+
@@ -36,6 +37,8 @@
35+
*/
36+
public final class PersistableBundle extends BaseBundle implements Cloneable, Parcelable,
37+
XmlUtils.WriteMapCallback {
38+
+ private static final String TAG = "PersistableBundle";
39+
+
40+
private static final String TAG_PERSISTABLEMAP = "pbundle_as_map";
41+
public static final PersistableBundle EMPTY;
42+
43+
@@ -95,7 +98,11 @@ public PersistableBundle(PersistableBundle b) {
44+
* @hide
45+
*/
46+
public PersistableBundle(Bundle b) {
47+
- this(b.getMap());
48+
+ this(b, true);
49+
+ }
50+
+
51+
+ private PersistableBundle(Bundle b, boolean throwException) {
52+
+ this(b.getMap(), throwException);
53+
}
54+
55+
/**
56+
@@ -104,7 +111,7 @@ public PersistableBundle(Bundle b) {
57+
* @param map a Map containing only those items that can be persisted.
58+
* @throws IllegalArgumentException if any element of #map cannot be persisted.
59+
*/
60+
- private PersistableBundle(ArrayMap<String, Object> map) {
61+
+ private PersistableBundle(ArrayMap<String, Object> map, boolean throwException) {
62+
super();
63+
mFlags = FLAG_DEFUSABLE;
64+
65+
@@ -113,16 +120,23 @@ private PersistableBundle(ArrayMap<String, Object> map) {
66+
67+
// Now verify each item throwing an exception if there is a violation.
68+
final int N = mMap.size();
69+
- for (int i=0; i<N; i++) {
70+
+ for (int i = N - 1; i >= 0; --i) {
71+
Object value = mMap.valueAt(i);
72+
if (value instanceof ArrayMap) {
73+
// Fix up any Maps by replacing them with PersistableBundles.
74+
- mMap.setValueAt(i, new PersistableBundle((ArrayMap<String, Object>) value));
75+
+ mMap.setValueAt(i,
76+
+ new PersistableBundle((ArrayMap<String, Object>) value, throwException));
77+
} else if (value instanceof Bundle) {
78+
- mMap.setValueAt(i, new PersistableBundle(((Bundle) value)));
79+
+ mMap.setValueAt(i, new PersistableBundle((Bundle) value, throwException));
80+
} else if (!isValidType(value)) {
81+
- throw new IllegalArgumentException("Bad value in PersistableBundle key="
82+
- + mMap.keyAt(i) + " value=" + value);
83+
+ final String errorMsg = "Bad value in PersistableBundle key="
84+
+ + mMap.keyAt(i) + " value=" + value;
85+
+ if (throwException) {
86+
+ throw new IllegalArgumentException(errorMsg);
87+
+ } else {
88+
+ Slog.wtfStack(TAG, errorMsg);
89+
+ mMap.removeAt(i);
90+
+ }
91+
}
92+
}
93+
}
94+
@@ -217,6 +231,15 @@ public void writeUnknownObject(Object v, String name, XmlSerializer out)
95+
/** @hide */
96+
public void saveToXml(XmlSerializer out) throws IOException, XmlPullParserException {
97+
unparcel();
98+
+ // Explicitly drop invalid types an attacker may have added before persisting.
99+
+ for (int i = mMap.size() - 1; i >= 0; --i) {
100+
+ final Object value = mMap.valueAt(i);
101+
+ if (!isValidType(value)) {
102+
+ Slog.e(TAG, "Dropping bad data before persisting: "
103+
+ + mMap.keyAt(i) + "=" + value);
104+
+ mMap.removeAt(i);
105+
+ }
106+
+ }
107+
XmlUtils.writeMapXml(mMap, out, this);
108+
}
109+
110+
@@ -265,9 +288,12 @@ public static PersistableBundle restoreFromXml(XmlPullParser in) throws IOExcept
111+
while (((event = in.next()) != XmlPullParser.END_DOCUMENT) &&
112+
(event != XmlPullParser.END_TAG || in.getDepth() < outerDepth)) {
113+
if (event == XmlPullParser.START_TAG) {
114+
+ // Don't throw an exception when restoring from XML since an attacker could try to
115+
+ // input invalid data in the persisted file.
116+
return new PersistableBundle((ArrayMap<String, Object>)
117+
XmlUtils.readThisArrayMapXml(in, startTag, tagName,
118+
- new MyReadMapCallback()));
119+
+ new MyReadMapCallback()),
120+
+ /* throwException */ false);
121+
}
122+
}
123+
return EMPTY;
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
From a341bc7b7ae161f4a87e996d8a5e8b1ad005fb6b Mon Sep 17 00:00:00 2001
2+
From: Pinyao Ting <[email protected]>
3+
Date: Mon, 24 Jul 2023 14:58:56 -0700
4+
Subject: [PATCH] Validate userId when publishing shortcuts
5+
6+
Bug: 288110451
7+
Test: manual
8+
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:01bfd04ff445db6290ae430d44ea1bf1a115fe3c)
9+
Merged-In: Idbde676f871db83825155730e3714f3727e25762
10+
Change-Id: Idbde676f871db83825155730e3714f3727e25762
11+
---
12+
services/core/java/com/android/server/pm/ShortcutService.java | 4 ++++
13+
1 file changed, 4 insertions(+)
14+
15+
diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java
16+
index 944f75345df6f..2cfc3461c6977 100644
17+
--- a/services/core/java/com/android/server/pm/ShortcutService.java
18+
+++ b/services/core/java/com/android/server/pm/ShortcutService.java
19+
@@ -1528,6 +1528,10 @@ private void verifyShortcutInfoPackage(String callerPackage, ShortcutInfo si) {
20+
android.util.EventLog.writeEvent(0x534e4554, "109824443", -1, "");
21+
throw new SecurityException("Shortcut package name mismatch");
22+
}
23+
+ final int callingUid = injectBinderCallingUid();
24+
+ if (UserHandle.getUserId(callingUid) != si.getUserId()) {
25+
+ throw new SecurityException("User-ID in shortcut doesn't match the caller");
26+
+ }
27+
}
28+
29+
private void verifyShortcutInfoPackages(
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
From cb95e01ba40c3b8c70f5810efb7abfd85bfd0b1f Mon Sep 17 00:00:00 2001
2+
From: Kunal Malhotra <[email protected]>
3+
Date: Thu, 2 Feb 2023 23:48:27 +0000
4+
Subject: [PATCH] Adding in verification of calling UID in onShellCommand
5+
6+
Test: manual testing on device
7+
Bug: b/261709193
8+
(cherry picked from commit b651d295b44eb82d664861b77f33dbde1bce9453)
9+
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3ef3f18ba3094c4cc4f954ba23d1da421f9ca8b0)
10+
Merged-In: I68903ebd6d3d85f4bc820b745e3233a448b62273
11+
Change-Id: I68903ebd6d3d85f4bc820b745e3233a448b62273
12+
---
13+
.../java/com/android/server/am/ActivityManagerService.java | 7 +++++++
14+
1 file changed, 7 insertions(+)
15+
16+
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
17+
index 4e48f422a2fe3..7cda7571df70d 100644
18+
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
19+
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
20+
@@ -14349,6 +14349,13 @@ public int getMemoryTrimLevel() {
21+
@Override
22+
public void onShellCommand(FileDescriptor in, FileDescriptor out,
23+
FileDescriptor err, String[] args, ResultReceiver resultReceiver) {
24+
+ final int callingUid = Binder.getCallingUid();
25+
+ if (callingUid != Process.ROOT_UID && callingUid != Process.SHELL_UID) {
26+
+ if (resultReceiver != null) {
27+
+ resultReceiver.send(-1, null);
28+
+ }
29+
+ throw new SecurityException("Shell commands are only callable by root or shell");
30+
+ }
31+
(new ActivityManagerShellCommand(this, false)).exec(
32+
this, in, out, err, args, resultReceiver);
33+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
From fd90a5342af9b9ead4026a70a694d3f31908d5ea Mon Sep 17 00:00:00 2001
2+
From: Brian Delwiche <[email protected]>
3+
Date: Thu, 5 Oct 2023 00:01:03 +0000
4+
Subject: [PATCH] Fix UAF in ~CallbackEnv
5+
6+
com_android_bluetooth_btservice_AdapterService does not null its local
7+
JNI environment variable after detaching the thread (which frees the
8+
environment context), allowing UAF under certain conditions.
9+
10+
Null the variable in this case.
11+
12+
Testing here was done through a custom unit test; see patchsets 4-6 for
13+
contents. However, unit testing of the JNI layer is problematic in
14+
production, so that part of the patch is omitted for final merge.
15+
16+
Bug: 291500341
17+
Test: atest bluetooth_test_gd_unit, atest net_test_stack_btm
18+
Tag: #security
19+
Ignore-AOSP-First: Security
20+
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5f543d919c4067f2f4925580fd8a690ba3440e80)
21+
Merged-In: I3e5e3c51412640aa19f0981caaa809313d6ad030
22+
Change-Id: I3e5e3c51412640aa19f0981caaa809313d6ad030
23+
---
24+
jni/com_android_bluetooth_btservice_AdapterService.cpp | 1 +
25+
1 file changed, 1 insertion(+)
26+
27+
diff --git a/jni/com_android_bluetooth_btservice_AdapterService.cpp b/jni/com_android_bluetooth_btservice_AdapterService.cpp
28+
index 1768ef51b..4a579e0ca 100644
29+
--- a/jni/com_android_bluetooth_btservice_AdapterService.cpp
30+
+++ b/jni/com_android_bluetooth_btservice_AdapterService.cpp
31+
@@ -465,6 +465,7 @@ static void callback_thread_event(bt_cb_thread_evt event) {
32+
return;
33+
}
34+
vm->DetachCurrentThread();
35+
+ callbackEnv = NULL;
36+
}
37+
}
38+
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
From e54bad92576d31fc959342e4c35d73abaf29d926 Mon Sep 17 00:00:00 2001
2+
From: balakrishna <[email protected]>
3+
Date: Tue, 7 Mar 2023 16:53:46 +0530
4+
Subject: [PATCH] BT: Fixing the rfc_slot_id overflow
5+
6+
Root cause:
7+
overflow causing leak in slot fds.
8+
As slot id 0 not valid, we are not able to release these fds later.
9+
10+
Fix:
11+
Changes are made to avoid overflow while allocate rfc slots.
12+
13+
CRs-Fixed: 3417458
14+
Change-Id: I5d7efa34bfb97a6dd8e9d68615d29120a0ae51f0
15+
---
16+
btif/src/btif_sock_rfc.c | 5 ++++-
17+
1 file changed, 4 insertions(+), 1 deletion(-)
18+
19+
diff --git a/btif/src/btif_sock_rfc.c b/btif/src/btif_sock_rfc.c
20+
index d5522b739eb..d77c3f44253 100644
21+
--- a/btif/src/btif_sock_rfc.c
22+
+++ b/btif/src/btif_sock_rfc.c
23+
@@ -225,8 +225,11 @@ static rfc_slot_t *alloc_rfc_slot(const bt_bdaddr_t *addr, const char *name, con
24+
}
25+
26+
// Increment slot id and make sure we don't use id=0.
27+
- if (++rfc_slot_id == 0)
28+
+ if (UINT32_MAX == rfc_slot_id) {
29+
rfc_slot_id = 1;
30+
+ } else {
31+
+ ++rfc_slot_id;
32+
+ }
33+
34+
slot->fd = fds[0];
35+
slot->app_fd = fds[1];
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
From e446be9bf42b2559add74d48b91bae52b828e24f Mon Sep 17 00:00:00 2001
2+
From: balakrishna <[email protected]>
3+
Date: Wed, 24 May 2023 13:28:21 +0530
4+
Subject: [PATCH] Fix OOB Write in pin_reply in bluetooth.cc
5+
6+
Root cause:
7+
if the length of "pin_code" is greater than 16,
8+
an OOBW will be triggered due to a missing bounds check.
9+
10+
Fix:
11+
Check is added to avoid Out of Bound Write.
12+
13+
CRs-Fixed: 3507292
14+
Change-Id: I15a1eae59b17f633e29180a01676c260189b8353
15+
---
16+
btif/src/bluetooth.c | 2 ++
17+
1 file changed, 2 insertions(+)
18+
19+
diff --git a/btif/src/bluetooth.c b/btif/src/bluetooth.c
20+
index d2f81733da4..5fc6c880db0 100644
21+
--- a/btif/src/bluetooth.c
22+
+++ b/btif/src/bluetooth.c
23+
@@ -345,6 +345,8 @@ static int pin_reply(const bt_bdaddr_t *bd_addr, uint8_t accept,
24+
/* sanity check */
25+
if (interface_ready() == FALSE)
26+
return BT_STATUS_NOT_READY;
27+
+ if (pin_code == NULL || pin_len > PIN_CODE_LEN)
28+
+ return BT_STATUS_FAIL;
29+
30+
return btif_dm_pin_reply(bd_addr, accept, pin_len, pin_code);
31+
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
From 277464e315d43f3d66e2444744dfa9e42977c64c Mon Sep 17 00:00:00 2001
2+
From: Hui Peng <[email protected]>
3+
Date: Sat, 2 Sep 2023 04:20:10 +0000
4+
Subject: [PATCH] Reject access to secure service authenticated from a temp
5+
bonding [1]
6+
7+
Rejecct access to services running on l2cap
8+
9+
Backport of
10+
Idef4ea28eb3d17b0807ab7dc6849433ddc5581b3
11+
12+
Bug: 294854926
13+
Test: m com.android.btservices
14+
Ignore-AOSP-First: security
15+
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a36757e967ab6d956127cac298134f28ce8f0d6d)
16+
Merged-In: Idef4ea28eb3d17b0807ab7dc6849433ddc5581b3
17+
Change-Id: Idef4ea28eb3d17b0807ab7dc6849433ddc5581b3
18+
---
19+
stack/btm/btm_sec.c | 41 +++++++++++++++++++++++++++++++++++++----
20+
1 file changed, 37 insertions(+), 4 deletions(-)
21+
22+
diff --git a/stack/btm/btm_sec.c b/stack/btm/btm_sec.c
23+
index b27b7e071c7..23b334ce27f 100644
24+
--- a/stack/btm/btm_sec.c
25+
+++ b/stack/btm/btm_sec.c
26+
@@ -106,7 +106,7 @@ static BOOLEAN btm_sec_set_security_level ( CONNECTION_TYPE conn_type, char *p_
27+
UINT16 sec_level, UINT16 psm, UINT32 mx_proto_id,
28+
UINT32 mx_chan_id);
29+
30+
-static BOOLEAN btm_dev_authenticated(tBTM_SEC_DEV_REC *p_dev_rec);
31+
+static BOOLEAN btm_dev_authenticated(const tBTM_SEC_DEV_REC* p_dev_rec);
32+
static BOOLEAN btm_dev_encrypted(tBTM_SEC_DEV_REC *p_dev_rec);
33+
static BOOLEAN btm_dev_authorized(tBTM_SEC_DEV_REC *p_dev_rec);
34+
static BOOLEAN btm_serv_trusted(tBTM_SEC_DEV_REC *p_dev_rec, tBTM_SEC_SERV_REC *p_serv_rec);
35+
@@ -145,7 +145,7 @@ static const BOOLEAN btm_sec_io_map [BTM_IO_CAP_MAX][BTM_IO_CAP_MAX] =
36+
** Returns BOOLEAN TRUE or FALSE
37+
**
38+
*******************************************************************************/
39+
-static BOOLEAN btm_dev_authenticated (tBTM_SEC_DEV_REC *p_dev_rec)
40+
+static BOOLEAN btm_dev_authenticated(const tBTM_SEC_DEV_REC* p_dev_rec)
41+
{
42+
if(p_dev_rec->sec_flags & BTM_SEC_AUTHENTICATED)
43+
{
44+
@@ -229,6 +229,26 @@ static BOOLEAN btm_serv_trusted(tBTM_SEC_DEV_REC *p_dev_rec, tBTM_SEC_SERV_REC *
45+
return(FALSE);
46+
}
47+
48+
+/*******************************************************************************
49+
+**
50+
+** Function access_secure_service_from_temp_bond
51+
+**
52+
+** Description a utility function to test whether an access to
53+
+** secure service from temp bonding is happening
54+
+**
55+
+** Returns true if the aforementioned condition holds,
56+
+** false otherwise
57+
+**
58+
+*******************************************************************************/
59+
+static BOOLEAN access_secure_service_from_temp_bond(const tBTM_SEC_DEV_REC* p_dev_rec,
60+
+ bool locally_initiated,
61+
+ uint16_t security_req)
62+
+{
63+
+ return !locally_initiated && (security_req & BTM_SEC_IN_AUTHENTICATE) &&
64+
+ btm_dev_authenticated(p_dev_rec) &&
65+
+ p_dev_rec->bond_type == BOND_TYPE_TEMPORARY;
66+
+}
67+
+
68+
/*******************************************************************************
69+
**
70+
** Function BTM_SecRegister
71+
@@ -2215,10 +2235,15 @@ tBTM_STATUS btm_sec_l2cap_access_req (BD_ADDR bd_addr, UINT16 psm, UINT16 handle
72+
73+
if (rc == BTM_SUCCESS)
74+
{
75+
+ if (access_secure_service_from_temp_bond(p_dev_rec, is_originator, security_required))
76+
+ {
77+
+ LOG_ERROR(LOG_TAG, "Trying to access a secure service from a temp bonding, rejecting");
78+
+ rc = BTM_FAILED_ON_SECURITY;
79+
+ }
80+
if (p_callback)
81+
- (*p_callback) (bd_addr, transport, (void *)p_ref_data, BTM_SUCCESS);
82+
+ (*p_callback)(&bd_addr, transport, (void*)p_ref_data, rc);
83+
84+
- return(BTM_SUCCESS);
85+
+ return (rc);
86+
}
87+
}
88+
else
89+
@@ -5587,6 +5612,14 @@ extern tBTM_STATUS btm_sec_execute_procedure (tBTM_SEC_DEV_REC *p_dev_rec)
90+
}
91+
}
92+
93+
+ if (access_secure_service_from_temp_bond(p_dev_rec,
94+
+ p_dev_rec->is_originator,
95+
+ p_dev_rec->security_required))
96+
+ {
97+
+ LOG_ERROR(LOG_TAG, "Trying to access a secure service from a temp bonding, rejecting");
98+
+ return (BTM_FAILED_ON_SECURITY);
99+
+ }
100+
+
101+
/* All required security procedures already established */
102+
p_dev_rec->security_required &= ~(BTM_SEC_OUT_AUTHORIZE | BTM_SEC_IN_AUTHORIZE |
103+
BTM_SEC_OUT_AUTHENTICATE | BTM_SEC_IN_AUTHENTICATE |

0 commit comments

Comments
 (0)