From e738188cd81dc63c415efcb06370746aa5684463 Mon Sep 17 00:00:00 2001 From: Ben Tian Date: Thu, 21 May 2015 14:11:20 +0800 Subject: [PATCH] Bug 1166587 - Check OBEX packet length before accessing it, r=shuang --- dom/bluetooth/ObexBase.h | 3 +- .../bluedroid/BluetoothOppManager.cpp | 38 ++++++++++++++++--- .../bluedroid/BluetoothPbapManager.cpp | 24 +++++++++--- dom/bluetooth/bluez/BluetoothOppManager.cpp | 38 ++++++++++++++++--- 4 files changed, 83 insertions(+), 20 deletions(-) diff --git a/dom/bluetooth/ObexBase.h b/dom/bluetooth/ObexBase.h index a2850ba4616..70c478a10ee 100644 --- a/dom/bluetooth/ObexBase.h +++ b/dom/bluetooth/ObexBase.h @@ -126,7 +126,7 @@ public: class ObexHeaderSet { public: - ObexHeaderSet(uint8_t aOpcode) : mOpcode(aOpcode) + ObexHeaderSet() { } @@ -252,7 +252,6 @@ public: } private: - uint8_t mOpcode; nsTArray > mHeaders; }; diff --git a/dom/bluetooth/bluedroid/BluetoothOppManager.cpp b/dom/bluetooth/bluedroid/BluetoothOppManager.cpp index 1bc513c3b14..e44df334109 100644 --- a/dom/bluetooth/bluedroid/BluetoothOppManager.cpp +++ b/dom/bluetooth/bluedroid/BluetoothOppManager.cpp @@ -899,10 +899,19 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) { MOZ_ASSERT(NS_IsMainThread()); - uint8_t opCode; + /** + * Ensure + * - valid access to data[0], i.e., opCode + * - received packet length smaller than max packet length + */ int receivedLength = aMessage->GetSize(); - const uint8_t* data = aMessage->GetData(); + if (receivedLength < 1 || receivedLength > MAX_PACKET_LENGTH) { + ReplyError(ObexResponseCode::BadRequest); + return; + } + uint8_t opCode; + const uint8_t* data = aMessage->GetData(); if (mPutPacketReceivedLength > 0) { opCode = mPutFinalFlag ? ObexRequestCode::PutFinal : ObexRequestCode::Put; } else { @@ -918,12 +927,13 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) } } - ObexHeaderSet pktHeaders(opCode); + ObexHeaderSet pktHeaders; if (opCode == ObexRequestCode::Connect) { // Section 3.3.1 "Connect", IrOBEX 1.2 // [opcode:1][length:2][version:1][flags:1][MaxPktSizeWeCanReceive:2] // [Headers:var] - if (!ParseHeaders(&data[7], receivedLength - 7, &pktHeaders)) { + if (receivedLength < 7 || + !ParseHeaders(&data[7], receivedLength - 7, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -933,7 +943,8 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) } else if (opCode == ObexRequestCode::Abort) { // Section 3.3.5 "Abort", IrOBEX 1.2 // [opcode:1][length:2][Headers:var] - if (!ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { + if (receivedLength < 3 || + !ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -943,7 +954,8 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) } else if (opCode == ObexRequestCode::Disconnect) { // Section 3.3.2 "Disconnect", IrOBEX 1.2 // [opcode:1][length:2][Headers:var] - if (!ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { + if (receivedLength < 3 || + !ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -1026,6 +1038,13 @@ BluetoothOppManager::ClientDataHandler(UnixSocketBuffer* aMessage) { MOZ_ASSERT(NS_IsMainThread()); + // Ensure valid access to data[0], i.e., opCode + int receivedLength = aMessage->GetSize(); + if (receivedLength < 1) { + BT_LOGR("Receive empty response packet"); + return; + } + const uint8_t* data = aMessage->GetData(); uint8_t opCode = data[0]; @@ -1083,6 +1102,13 @@ BluetoothOppManager::ClientDataHandler(UnixSocketBuffer* aMessage) AfterOppConnected(); + // Ensure valid access to remote information + if (receivedLength < 7) { + BT_LOGR("The length of connect response packet is invalid"); + SendDisconnectRequest(); + return; + } + // Keep remote information mRemoteObexVersion = data[3]; mRemoteConnectionFlags = data[4]; diff --git a/dom/bluetooth/bluedroid/BluetoothPbapManager.cpp b/dom/bluetooth/bluedroid/BluetoothPbapManager.cpp index 7c429dc875a..5aac14a3ed2 100644 --- a/dom/bluetooth/bluedroid/BluetoothPbapManager.cpp +++ b/dom/bluetooth/bluedroid/BluetoothPbapManager.cpp @@ -183,17 +183,27 @@ BluetoothPbapManager::ReceiveSocketData(BluetoothSocket* aSocket, { MOZ_ASSERT(NS_IsMainThread()); - const uint8_t* data = aMessage->GetData(); + /** + * Ensure + * - valid access to data[0], i.e., opCode + * - received packet length smaller than max packet length + */ int receivedLength = aMessage->GetSize(); - uint8_t opCode = data[0]; + if (receivedLength < 1 || receivedLength > MAX_PACKET_LENGTH) { + ReplyError(ObexResponseCode::BadRequest); + return; + } - ObexHeaderSet pktHeaders(opCode); + const uint8_t* data = aMessage->GetData(); + uint8_t opCode = data[0]; + ObexHeaderSet pktHeaders; switch (opCode) { case ObexRequestCode::Connect: // Section 3.3.1 "Connect", IrOBEX 1.2 // [opcode:1][length:2][version:1][flags:1][MaxPktSizeWeCanReceive:2] // [Headers:var] - if (!ParseHeaders(&data[7], receivedLength - 7, &pktHeaders)) { + if (receivedLength < 7 || + !ParseHeaders(&data[7], receivedLength - 7, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -213,7 +223,8 @@ BluetoothPbapManager::ReceiveSocketData(BluetoothSocket* aSocket, // Section 3.3.2 "Disconnect" and Section 3.3.5 "Abort", IrOBEX 1.2 // The format of request packet of "Disconnect" and "Abort" are the same // [opcode:1][length:2][Headers:var] - if (!ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { + if (receivedLength < 3 || + !ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -224,7 +235,8 @@ BluetoothPbapManager::ReceiveSocketData(BluetoothSocket* aSocket, case ObexRequestCode::SetPath: { // Section 3.3.6 "SetPath", IrOBEX 1.2 // [opcode:1][length:2][flags:1][contants:1][Headers:var] - if (!ParseHeaders(&data[5], receivedLength - 5, &pktHeaders)) { + if (receivedLength < 5 || + !ParseHeaders(&data[5], receivedLength - 5, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } diff --git a/dom/bluetooth/bluez/BluetoothOppManager.cpp b/dom/bluetooth/bluez/BluetoothOppManager.cpp index 69a9a421fa2..d80d07f174a 100644 --- a/dom/bluetooth/bluez/BluetoothOppManager.cpp +++ b/dom/bluetooth/bluez/BluetoothOppManager.cpp @@ -878,10 +878,19 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) { MOZ_ASSERT(NS_IsMainThread()); - uint8_t opCode; + /** + * Ensure + * - valid access to data[0], i.e., opCode + * - received packet length smaller than max packet length + */ int receivedLength = aMessage->GetSize(); - const uint8_t* data = aMessage->GetData(); + if (receivedLength < 1 || receivedLength > MAX_PACKET_LENGTH) { + ReplyError(ObexResponseCode::BadRequest); + return; + } + uint8_t opCode; + const uint8_t* data = aMessage->GetData(); if (mPutPacketReceivedLength > 0) { opCode = mPutFinalFlag ? ObexRequestCode::PutFinal : ObexRequestCode::Put; } else { @@ -897,12 +906,13 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) } } - ObexHeaderSet pktHeaders(opCode); + ObexHeaderSet pktHeaders; if (opCode == ObexRequestCode::Connect) { // Section 3.3.1 "Connect", IrOBEX 1.2 // [opcode:1][length:2][version:1][flags:1][MaxPktSizeWeCanReceive:2] // [Headers:var] - if (!ParseHeaders(&data[7], receivedLength - 7, &pktHeaders)) { + if (receivedLength < 7 || + !ParseHeaders(&data[7], receivedLength - 7, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -912,7 +922,8 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) } else if (opCode == ObexRequestCode::Abort) { // Section 3.3.5 "Abort", IrOBEX 1.2 // [opcode:1][length:2][Headers:var] - if (!ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { + if (receivedLength < 3 || + !ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -922,7 +933,8 @@ BluetoothOppManager::ServerDataHandler(UnixSocketBuffer* aMessage) } else if (opCode == ObexRequestCode::Disconnect) { // Section 3.3.2 "Disconnect", IrOBEX 1.2 // [opcode:1][length:2][Headers:var] - if (!ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { + if (receivedLength < 3 || + !ParseHeaders(&data[3], receivedLength - 3, &pktHeaders)) { ReplyError(ObexResponseCode::BadRequest); return; } @@ -1005,6 +1017,13 @@ BluetoothOppManager::ClientDataHandler(UnixSocketBuffer* aMessage) { MOZ_ASSERT(NS_IsMainThread()); + // Ensure valid access to data[0], i.e., opCode + int receivedLength = aMessage->GetSize(); + if (receivedLength < 1) { + BT_LOGR("Receive empty response packet"); + return; + } + const uint8_t* data = aMessage->GetData(); uint8_t opCode = data[0]; @@ -1062,6 +1081,13 @@ BluetoothOppManager::ClientDataHandler(UnixSocketBuffer* aMessage) AfterOppConnected(); + // Ensure valid access to remote information + if (receivedLength < 7) { + BT_LOGR("The length of connect response packet is invalid"); + SendDisconnectRequest(); + return; + } + // Keep remote information mRemoteObexVersion = data[3]; mRemoteConnectionFlags = data[4];