Closed Bug 949843 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, firefox-esr24 unaffected, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 unaffected, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

RESOLVED FIXED
1.3 C2/1.4 S2(17jan)
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: m_kato, Assigned: ben.tian)

References

Details

(Keywords: sec-critical, Whiteboard: [adv-main28+])

Attachments

(1 file)

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();
Assignee: nobody → m_kato
Assignee: m_kato → nobody
Component: DOM → Bluetooth
Product: Core → Firefox OS
Version: Trunk → unspecified
Thanks for reporting. I've talked to Ben and he will take this over.
Assignee: nobody → btian
Attachment #8347861 - Flags: review?(echou)
Attachment #8347861 - Flags: feedback?(m_kato)
Attachment #8347861 - Flags: feedback?(m_kato) → feedback+
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.
Attachment #8347861 - Flags: review?(echou) → review+
Nominate as 1.3+ since 1.3 would be affected.
blocking-b2g: --- → 1.3?
Attachment #8347861 - Attachment description: Patch 1 (v1): Declare local UTF8 string variable → [final] Patch 1: Declare local UTF8 string variable, r=echou
landed on central:

https://hg.mozilla.org/mozilla-central/rev/e769b355870f
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Paul - Can you provide input from a security perspective if this is a blocker for 1.3?
Flags: needinfo?(ptheriault)
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
Flags: needinfo?(ptheriault)
Keywords: sec-critical
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.
blocking-b2g: 1.3? → 1.3+
(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.
Keywords: checkin-needed
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
Keywords: checkin-needed
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Pretty sure bluedroid wasnt added until 1.3. (And I can't see this file in the 1.2 source repo)
Whiteboard: [adv-main28+]
Does it impact also ESR24?
Isn't this b2g only?
(In reply to Al Billings [:abillings] from comment #15)
> Isn't this b2g only?

Yes.
Group: core-security
You need to log in before you can comment on or make changes to this bug.