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.

Sign up for GitHub

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

Jump to bottom

6 months worth of security patches! #1

Open
wants to merge 12 commits into
base: 12.1
Choose a base branch
from
Open

6 months worth of security patches! #1

wants to merge 12 commits into from

Commits on Aug 6, 2023

  1. Report failure when not able to connect to AVRCP 

    A crash may occur when creating a bluetooth AVRCP connection to a
device.

The code fails to check a return value from an AVRCP function
being used to index into an array. The return value may exceed the
size of the array causing memory outside the bounds of the array to be
accessed leading to memory corruption and a crash.

The fix is to ensure the return value is within the bounds of the
array before accessing the array contents. If the return value is
not within the bounds of the array report it as a failure to the
bluetooth stack.

This change is relevant for android automotive because the IVI
(in-vehicle infotainment system) acts as the an AVRCP controller
which still executes this code.

Note: this is a backport of b/214569798, inducted as a non-security
issue.  Per b/226927612 it has been found to have security impact
and should be backported to earlier branches.

Bug: 226927612
Test: Manual - set return value to be out of bounds, verify no crash
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I03f89f894c759b85e555a024435b625397ef7e5c
(cherry picked from commit 6a543761f2dc3db0ebf541285a0b3b2afc83a6a6)
Merged-In: I03f89f894c759b85e555a024435b625397ef7e5c
    @Meghthedev
    Brian Delwiche authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    6588046 View commit details
    Browse the repository at this point in the history

  2. Fix an OOB Write bug in gatt_check_write_long_terminate 

    this is the backport of Ifffa2c7f679c4ef72dbdb6b1f3378ca506680084

Bug: 258652631
Test: manual
Tag: #security
Ignore-AOSP-First: security
Change-Id: Ic84122f07cbc198c676d366e39606621b7cb4e66
(cherry picked from commit 9b17660bfd6f0f41cb9400ce0236d76c83605e03)
Merged-In: Ic84122f07cbc198c676d366e39606621b7cb4e66
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    7e3abb6 View commit details
    Browse the repository at this point in the history

  3. Fix an OOB access bug in A2DP_BuildMediaPayloadHeaderSbc 

    In  A2DP_BuildCodecHeaderSbc when p_buf->offset is 0, the
`-=` operation on it may result in integer underflow and
OOB write with the computed pointer passed to
A2DP_BuildMediaPayloadHeaderSbc.

This is a backport of I45320085b1e458d3b0e0d86162a35aaaae7b34cb
Test: atest net_test_stack_a2dp_codecs_native
Ignore-AOSP-First: security
Tag:#security

Bug: 186803518
Change-Id: I4ff1a1de71884b8de23008b2569fdea3650e85ec
(cherry picked from commit a710300216be4a86373a65c6a685aeef8509cfa7)
Merged-In: I4ff1a1de71884b8de23008b2569fdea3650e85ec
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    e70015e View commit details
    Browse the repository at this point in the history

  4. Fix an OOB write in SDP_AddAttribute 

    When the `attr_pad` becomes full, it is possible
that un index of `-1` is computed write
a zero byte to `p_val`, rusulting OOB write.

```
  p_val[SDP_MAX_PAD_LEN - p_rec->free_pad_ptr - 1] = '\0';
```

This is a backport of I937d22a2df26fca1d7f06b10182c4e713ddfed1b

Bug: 261867748
Test: manual
Tag: #security
Ignore-AOSP-First: security
Change-Id: Ibdda754e628cfc9d1706c14db114919a15d8d6b1
(cherry picked from commit cc527a97f78a2999a0156a579e488afe9e3675b2)
Merged-In: Ibdda754e628cfc9d1706c14db114919a15d8d6b1
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    4ff4154 View commit details
    Browse the repository at this point in the history

  5. Fix OOB access in avdt_scb_hdl_pkt_no_frag 

    This is a back port of the following 2 CLs:
- Id13b1ebde8f603123c8b7a49922b2f1378ab788f
- If0c7b25f2e6cb4531bbb6254e176e8ad1b5c5fb4

Regression test: I9c87e30ed58e7ad6a34ab7c96b0a8fb06324ad54

Bug: 142546355 258057241
Test: atest net_test_stack_avdtp
Ignore-AOSP-First: security
Change-Id: Ie1707385d6452ece47915c153f4faaa1c8a287c9
(cherry picked from commit b0b968e8c6214e20a5dc3617d66567225df0884f)
Merged-In: Ie1707385d6452ece47915c153f4faaa1c8a287c9
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    52cfa0f View commit details
    Browse the repository at this point in the history

  6. Add mocking support for now function in AttributionProcessor 

    The triggering of the code we want to test depends on  the return
value of std::chrono::system_clock::now(). To facilicate testing,
in this patch we add a now_func_ field in
AttributionProcessor and make it call it instead of
std::chrono::system_clock::now(). Mocking `now` is made possible
by passing a custom function to the constructor of AttributionProcessor.

Note:
1. This is a manual cherrypick of I7dd3a0e665f72c27e4d1844f45ec15a8dd1ddb53

Bug: 254774758
Test: refactoring, existing tests still pass
Ignore-AOSP-First: security
Change-Id: I1526d794ddd86f53a189f3ff226bddbff7a487e5
(cherry picked from commit 7378585e68bdccd53e2b713d474229310f0672d0)
Merged-In: I1526d794ddd86f53a189f3ff226bddbff7a487e5
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    51e179b View commit details
    Browse the repository at this point in the history

  7. Add regression test for b/254774758 

    Note: this is a manual cherrypick of I1709af943b6fa238dd4df41a62e6add36984c9ec

Bug: 254774758
Ignore-AOSP-First: security
Test: atest bluetooth_test_gd_unit
Change-Id: If40eb63e00c1a97e15dcdfdbbf12fad1070cd97b
(cherry picked from commit 6059eb91d45d87a8e339bd0928a5837e6a96a324)
Merged-In: If40eb63e00c1a97e15dcdfdbbf12fad1070cd97b
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    cc41746 View commit details
    Browse the repository at this point in the history

  8. Fix an OOB bug in register_notification_rsp 

    This is a backport of I901d973a736678d7f3cc816ddf0cbbcbbd1fe93f
to rvc-dev.

Bug: 245916076
Test: manual
Ignore-AOSP-First: security
Change-Id: I37a9f45e707702b2ec52b5a2d572f177f2911765
(cherry picked from commit 901e34203c6280d414cbfa3978de04fd6515ffdf)
Merged-In: I37a9f45e707702b2ec52b5a2d572f177f2911765
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    2bb476b View commit details
    Browse the repository at this point in the history

  9. Fix a use-after-free bug in AttributionProcessor::OnWakelockReleased 

    There is a use-after-free bug in AttributionProcessor::OnWakelockReleased
resulted from a well-known misuse of using iterators to delete
items in containers (the deleted items are used for calculating the next iterator
in the next round). This patch fix it with correct usage.

Note:
1. This is a cherry-pick of  If9f14d5fe2fbf2150f2ab0d1f90ce0f263399227
2. The regression test is: If40eb63e00c1a97e15dcdfdbbf12fad1070cd97b

Bug: 254774758
Ignore-AOSP-First: security
Test: atest bluetooth_test_gd_unit
Change-Id: I75576e59e0c81a82473a68a6c5ba3ce882a84f99
(cherry picked from commit 9774aeff84a834ae4403300b5ef88f0a4635e9ac)
Merged-In: I75576e59e0c81a82473a68a6c5ba3ce882a84f99
    @benquike @Meghthedev
    benquike authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    a4b4a85 View commit details
    Browse the repository at this point in the history

  10. Prevent use-after-free of HID reports 

    BTA sends the the HID report pointer to BTIF and deallocates it immediately.
This is now prevented by providing a deep copy callback function for HID
reports when tranferring context from BTA to BTIF.

This is a backport of change Icef7a7ed1185b4283ee4fe4f812ca154d8f1b825,
already merged on T for b/227620181.

Bug: 228837201
Test: Validated against researcher POC, ran BT unit tests, played audio
manually.
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:874c495c886cd8722625756dc5fd0634b16b4f42)
Merged-In: Ib837f395883de2369207f1b3b974d6bff02dcb19
Change-Id: Ib837f395883de2369207f1b3b974d6bff02dcb19
    @Meghthedev
    Brian Delwiche authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    aab6246 View commit details
    Browse the repository at this point in the history

  11. Revert^2 "Validate buffer length in sdpu_build_uuid_seq" 

    fd2ded7341c7f867a153e86f003758808f11bfb9
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:4d33899d2c0573cf351691cdf27628416621f545)
Merged-In: I40ea9f3858215f460e6dab3768e0c6d2155e4755
Change-Id: I40ea9f3858215f460e6dab3768e0c6d2155e4755
    @Meghthedev
    Brian Delwiche authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    792d215 View commit details
    Browse the repository at this point in the history

  12. Revert "Revert "Fix wrong BR/EDR link key downgrades (P_256->P_192)"" 

    This reverts commit d733c86cbc06ce0ec72216b9d41e172d1939c46f.

Function btm_sec_encrypt_change() is called at most places
with argument "encr_enable" treated as bool and not as per
(tHCI_ENCRYPT_MODE = 0/1/2) expected by the function. The
function has special handling for "encr_enable=1" to downgrade
the link key type for BR/EDR case. This gets executed even
when the caller/context did not mean/expect so. It appears
this handling in btm_sec_encrypt_change() is not necessary and
is removed by this commit to prevent accidental execution of it.

Test: Verified re-pairing with an iPhone works fine now

Issue Reproduction Steps:
1. Enable Bluetooth Hotspot on Android device (DUT).
2. Pair and connect an iPhone to DUT.
3. Forget this pairing on DUT.
4. On iPhone settings, click on old DUT's paired entry to connect.
5. iPhone notifies to click 'Forget Device' and try fresh pairing.
6. On iPhone, after doing 'Forget Device', discover DUT again.
7. Attempt pairing to DUT by clicking on discovered DUT entry.
   Pairing will be unsuccessful.

Issue Cause:
During re-pairing, DUT is seen to downgrade
BR/EDR link key unexpectedly from link key type 0x8
(BTM_LKEY_TYPE_AUTH_COMB_P_256) to 0x5 (BTM_LKEY_TYPE_AUTH_COMB).

Log snippet (re-pairing time):
btm_sec_link_key_notification set new_encr_key_256 to 1
btif_dm_auth_cmpl_evt: Storing link key. key_type=0x8, bond_type=1
btm_sec_encrypt_change new_encr_key_256 is 1
--On DUT, HCI_Encryption_Key_Refresh_Complete event noticed---
btm_sec_encrypt_change new_encr_key_256 is 0
updated link key type to 5
btif_dm_auth_cmpl_evt: Storing link key. key_type=0x5, bond_type=1

This is a backport of the following patch: aosp/1890096

Bug: 258834033

Reason for revert: Reinstate original change for QPR
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:56891eedc68c86b40977191dad28d65ebf86a94f)
Merged-In: Iba0c220b82bcf6b15368762b7052a3987ccbc0c6
Change-Id: Iba0c220b82bcf6b15368762b7052a3987ccbc0c6
    @Meghthedev
    Brian Delwiche authored and Meghthedev committed Aug 6, 2023
    Configuration menu
    Copy the full SHA
    4e99477 View commit details
    Browse the repository at this point in the history