949843 - Don't use "char* str = NS_ConvertUTF16toUTF8(aBdAddress).get();"

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Makoto Kato [:m_kato]]

This means,

char *str;
{
  NS_ConvertUTF16toUTF8 tmp(aBdAddress);
  str = tmp;
}
... (use str) ...

so when using str, buffer is already freed.  This becomes use-after-free.


/dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp (View Hg log or Hg annotations)
    line 249 -- const char* str = NS_ConvertUTF16toUTF8(aBdAddress).get();
    line 1004 -- const char* name = NS_ConvertUTF16toUTF8(str).get();

Comment 1

[Eric Chou [:ericchou] [:echou]]

Thanks for reporting. I've talked to Ben and he will take this over.

Comment 2

[Ben Tian (inactive)]

Attachment: [final] Patch 1: Declare local UTF8 string variable, r=echou

Comment 3

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8347861 [details] [diff] [review]
[final] Patch 1: Declare local UTF8 string variable, r=echou

Review of attachment 8347861 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

Comment 4

[Eric Chou [:ericchou] [:echou]]

Nominate as 1.3+ since 1.3 would be affected.

Comment 5

[Ben Tian (inactive)]

try server: https://tbpl.mozilla.org/?tree=Try&rev=aceba6bd8795

Comment 6

[Ben Tian (inactive)]

https://hg.mozilla.org/integration/b2g-inbound/rev/e769b355870f

Comment 7

[Carsten Book [:Tomcat]]

landed on central:

https://hg.mozilla.org/mozilla-central/rev/e769b355870f

Comment 8

[Jason Smith [:jsmith]]

Paul - Can you provide input from a security perspective if this is a blocker for 1.3?

Comment 9

[Paul Theriault [:pauljt] (no longer reading bugmail)]

If this is remotely exploitable I would say this is sec-critical, otherwise maybe sec-high. From a very quick skim of this code, it seems that StringToBdAddressType is called using the remote address value[1], so this may be remotely exploitable, so I would err on the side of sec-critical.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp#1033

Comment 10

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Although that said, bluetooth would need to be enabled, and attacker would have to be local. So maybe it is a sec-high. Either way I think we need to get this on 1.3, especially since its already patched and the patch is public.

Comment 11

[Ben Tian (inactive)]

(In reply to Ben Tian [:btian] from comment #2)
> Created attachment 8347861 [details] [diff] [review]
> [final] Patch 1: Declare local UTF8 string variable, r=echou
Request to uplift the patch to 1.3.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1105948b00e8

FWIW, you don't need to explicitly request uplift. We have bug queries to assist with such things :)
https://wiki.mozilla.org/Release_Management/B2G_Landing

Comment 13

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Pretty sure bluedroid wasnt added until 1.3. (And I can't see this file in the 1.2 source repo)

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Does it impact also ESR24?

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Isn't this b2g only?

Comment 16

[Makoto Kato [:m_kato]]

(In reply to Al Billings [:abillings] from comment #15)
> Isn't this b2g only?

Yes.