Closed Bug 932968 Opened 8 years ago Closed 4 years ago

FxOS Bluetooth Null Dereferences

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: rfletcher, Unassigned)

Details

(Keywords: crash, csectype-dos, sec-low)

BluetoothHfpManager.cpp, BluetoothRilListener.cpp, BluetoothDbusService.cpp, and
BluetoothOppManager.cpp dereference potential null variables.

The goal of the bug is to determine if any of the dereferences have the
potential to cause a crash and to fix those instances.
********************************************************************************
BluetoothHfpManager::Get() can return nullptr via the macros at [1] and [2].
The following instances dereference the return value from Get(), which has the
potential to be null:

BluetoothHfpManager.cpp:[3]
* calls BluetoothHfpManager::Get() and dereferences result without checking for null.

BluetoothRilListener.cpp: [4], [5], [6], [7], [8], [9]
* calls BluetoothHfpManager::Get() and dereferences result without checking for null

BluetoothDBusService.cpp: [10], [11], [12]
* calls BluetoothHfpManager::Get() and dereferences result without checking for null

It might be the case that it is not necessary to check for null in some of the
instances, but Get() is definitely checked for returning null in several places
of the code so it is unclear.
********************************************************************************
BluetoothOppManager.cpp 
mSocket has the potential to be null at [13] and [14]

Much of the code inside BluetoothOppManager.cpp dereferences mSocket without
checking for null first; I will not list all of them.

It might not be necessary to check for null, but mSocket appears to have the
potential to be null in some of the dereferences.
********************************************************************************
BluetoothOppManager.cpp
mBlob has the potential to be null at [15]

Much of the code inside BluetoothOppManager.cpp dereferences mBlob without
checking for null first; I will not list all of them.

It might not be necessary to check for null, but mBlob appears to have the
potential to be null in some of the dereferences.
********************************************************************************

[1] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothHfpManager.cpp#448
[2] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothHfpManager.cpp#452
[3] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothHfpManager.cpp#163
[4] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothRilListener.cpp#38
[5] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothRilListener.cpp#79
[6] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothRilListener.cpp#151
[7] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothRilListener.cpp#167
[8] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothRilListener.cpp#177
[9] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothRilListener.cpp#217
[10] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/linux/BluetoothDBusService.cpp#2635
[11] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/linux/BluetoothDBusService.cpp#2646
[12] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/linux/BluetoothDBusService.cpp#2657
[13] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothOppManager.cpp#1475
[14] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothOppManager.cpp#1494
[15] http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothOppManager.cpp#849
We don't normally treat null derefs as security problems. If the values aren't null we'll use the pointer, and if THAT is bogus then we've got a real security issue -- but that would be the issue not the lack of the null check.

This could of course be a terrible stability problem depending on how likely nulls are
Keywords: crash, csec-dos
Right, more or less dchan and I arrived at the same conclusion but decided to err on the side of caution and file them in one large bug. There is definitely code that returns null objects that are also dereferenced. The unknown portion is if those code paths ever line up such that it is actually null when it is dereferenced.
Keywords: sec-low
Group: b2g-core-security
Group: b2g-core-security
Group: core-security → b2g-core-security
FirefoxOS is no longer under active development.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Group: b2g-core-security
You need to log in before you can comment on or make changes to this bug.