From f4bd0731fcaa851f6fb7ea49389313e9398501a9 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Sat, 10 Dec 2022 02:09:00 +0000
Subject: [PATCH] Fix potential use after free in pan_api.cc
Structure length is checked in pan_api.cc after the structure may
be freed, leading to a potential use after free.
Save the buffer length to a local instead. Note that BNEP_WriteBuf
may alter the length being written internally; this does not appear
to be an issue in this use case because the octet count being tracked
is used only for logging purposes within PAN.
Bug: 259939435
Test: atest bluetooth_test_gd_unit, validate against researcher POC
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I613b3dd3684182bdc725f9e1512061484448d367
---
system/stack/pan/pan_api.cc | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/system/stack/pan/pan_api.cc b/system/stack/pan/pan_api.cc
index 40b26a3acae..3989dacae06 100644
--- a/system/stack/pan/pan_api.cc
+++ b/system/stack/pan/pan_api.cc
@@ -509,6 +509,12 @@ tPAN_RESULT PAN_WriteBuf(uint16_t handle, const RawAddress& dst,
return PAN_FAILURE;
}
+ /* There are cases where BNAP_WriteBuf alters p_buf->len. However,
+ * the octets being handled are only used later by PAN for logging
+ * purposes, and for those purposes this length is arguably correct --
+ * it is the number of bytes handled at the PAN level. */
+ uint16_t bytes = p_buf->len;
+
result =
BNEP_WriteBuf(pan_cb.pcb[i].handle, dst, p_buf, protocol, &src, ext);
if (result == BNEP_IGNORE_CMD) {
@@ -519,7 +525,7 @@ tPAN_RESULT PAN_WriteBuf(uint16_t handle, const RawAddress& dst,
return (tPAN_RESULT)result;
}
- pan_cb.pcb[i].write.octets += p_buf->len;
+ pan_cb.pcb[i].write.octets += bytes;
pan_cb.pcb[i].write.packets++;
PAN_TRACE_DEBUG("PAN successfully wrote data for the PANU connection");
--
GitLab