Bug 1166587 - Check OBEX packet length before accessing it, r=shuang
authorBen Tian <btian@mozilla.com>
Thu, 21 May 2015 14:11:20 +0800
changeset 275626 fbeefb82d466a72c3e9ffa7eb479a9ede0236fe1
parent 275625 b29b0dfd8ac66924f52fb843aee0945a320e957f
child 275627 832f48c291f3514dea313c07d76f3a21ee50b687
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshuang
bugs1166587
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1166587 - Check OBEX packet length before accessing it, r=shuang
dom/bluetooth/ObexBase.h
dom/bluetooth/bluedroid/BluetoothOppManager.cpp
dom/bluetooth/bluedroid/BluetoothPbapManager.cpp
dom/bluetooth/bluez/BluetoothOppManager.cpp
--- a/dom/bluetooth/ObexBase.h
+++ b/dom/bluetooth/ObexBase.h
@@ -121,17 +121,17 @@ public:
   ObexHeaderId mId;
   int mDataLength;
   nsAutoArrayPtr<uint8_t> mData;
 };
 
 class ObexHeaderSet
 {
 public:
-  ObexHeaderSet(uint8_t aOpcode) : mOpcode(aOpcode)
+  ObexHeaderSet()
   {
   }
 
   ~ObexHeaderSet()
   {
   }
 
   void AddHeader(ObexHeader* aHeader)
@@ -247,17 +247,16 @@ public:
   }
 
   void ClearHeaders()
   {
     mHeaders.Clear();
   }
 
 private:
-  uint8_t mOpcode;
   nsTArray<nsAutoPtr<ObexHeader> > mHeaders;
 };
 
 int AppendHeaderName(uint8_t* aRetBuf, int aBufferSize, const uint8_t* aName,
                      int aLength);
 int AppendHeaderBody(uint8_t* aRetBuf, int aBufferSize, const uint8_t* aBody,
                      int aLength);
 int AppendHeaderWho(uint8_t* aRetBuf, int aBufferSize, const uint8_t* aWho,
--- a/dom/bluetooth/bluedroid/BluetoothOppManager.cpp
+++ b/dom/bluetooth/bluedroid/BluetoothOppManager.cpp
@@ -894,61 +894,73 @@ BluetoothOppManager::ComposePacket(uint8
   return (mPutPacketReceivedLength == mPacketLength);
 }
 
 void
 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();
+  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 {
     opCode = data[0];
 
     // When there's a Put packet right after a PutFinal packet,
     // which means it's the start point of a new file.
     if (mPutFinalFlag &&
         (opCode == ObexRequestCode::Put ||
          opCode == ObexRequestCode::PutFinal)) {
       mNewFileFlag = true;
       AfterFirstPut();
     }
   }
 
-  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;
     }
 
     ReplyToConnect();
     AfterOppConnected();
   } 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;
     }
 
     ReplyToDisconnectOrAbort();
     DeleteReceivedFile();
   } 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;
     }
 
     ReplyToDisconnectOrAbort();
     AfterOppDisconnected();
     FileTransferComplete();
   } else if (opCode == ObexRequestCode::Put ||
@@ -1021,16 +1033,23 @@ BluetoothOppManager::ServerDataHandler(U
   }
 }
 
 void
 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];
 
   // Check response code and send out system message as finished if the response
   // code is somehow incorrect.
   uint8_t expectedOpCode = ObexResponseCode::Success;
   if (mLastCommand == ObexRequestCode::Put) {
     expectedOpCode = ObexResponseCode::Continue;
@@ -1078,16 +1097,23 @@ BluetoothOppManager::ClientDataHandler(U
         PostDelayedTask(FROM_HERE, new CloseSocketTask(mSocket), 1000);
     }
   } else if (mLastCommand == ObexRequestCode::Connect) {
     MOZ_ASSERT(!mFileName.IsEmpty());
     MOZ_ASSERT(mBlob);
 
     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];
     mRemoteMaxPacketLength = ((static_cast<int>(data[5]) << 8) | data[6]);
 
     // The length of file name exceeds maximum length.
     int fileNameByteLen = (mFileName.Length() + 1) * 2;
     int headerLen = kPutRequestHeaderSize + kPutRequestAppendHeaderSize;
--- a/dom/bluetooth/bluedroid/BluetoothPbapManager.cpp
+++ b/dom/bluetooth/bluedroid/BluetoothPbapManager.cpp
@@ -178,27 +178,37 @@ BluetoothPbapManager::Listen()
 
 // Virtual function of class SocketConsumer
 void
 BluetoothPbapManager::ReceiveSocketData(BluetoothSocket* aSocket,
                                         nsAutoPtr<UnixSocketBuffer>& aMessage)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
+  /**
+   * Ensure
+   * - valid access to data[0], i.e., opCode
+   * - received packet length smaller than max packet length
+   */
+  int receivedLength = aMessage->GetSize();
+  if (receivedLength < 1 || receivedLength > MAX_PACKET_LENGTH) {
+    ReplyError(ObexResponseCode::BadRequest);
+    return;
+  }
+
   const uint8_t* data = aMessage->GetData();
-  int receivedLength = aMessage->GetSize();
   uint8_t opCode = data[0];
-
-  ObexHeaderSet pktHeaders(opCode);
+  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;
       }
 
       // Section 6.4 "Establishing an OBEX Session", PBAP 1.2
       // The OBEX header target shall equal to kPbapObexTarget.
       if (!CompareHeaderTarget(pktHeaders)) {
         ReplyError(ObexResponseCode::BadRequest);
@@ -208,28 +218,30 @@ BluetoothPbapManager::ReceiveSocketData(
       ReplyToConnect();
       AfterPbapConnected();
       break;
     case ObexRequestCode::Disconnect:
     case ObexRequestCode::Abort:
       // 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;
       }
 
       ReplyToDisconnectOrAbort();
       AfterPbapDisconnected();
       break;
     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;
         }
 
         uint8_t response = SetPhoneBookPath(data[3], pktHeaders);
         if (response != ObexResponseCode::Success) {
           ReplyError(response);
           return;
--- a/dom/bluetooth/bluez/BluetoothOppManager.cpp
+++ b/dom/bluetooth/bluez/BluetoothOppManager.cpp
@@ -873,61 +873,73 @@ BluetoothOppManager::ComposePacket(uint8
   return (mPutPacketReceivedLength == mPacketLength);
 }
 
 void
 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();
+  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 {
     opCode = data[0];
 
     // When there's a Put packet right after a PutFinal packet,
     // which means it's the start point of a new file.
     if (mPutFinalFlag &&
         (opCode == ObexRequestCode::Put ||
          opCode == ObexRequestCode::PutFinal)) {
       mNewFileFlag = true;
       AfterFirstPut();
     }
   }
 
-  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;
     }
 
     ReplyToConnect();
     AfterOppConnected();
   } 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;
     }
 
     ReplyToDisconnectOrAbort();
     DeleteReceivedFile();
   } 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;
     }
 
     ReplyToDisconnectOrAbort();
     AfterOppDisconnected();
     FileTransferComplete();
   } else if (opCode == ObexRequestCode::Put ||
@@ -1000,16 +1012,23 @@ BluetoothOppManager::ServerDataHandler(U
   }
 }
 
 void
 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];
 
   // Check response code and send out system message as finished if the response
   // code is somehow incorrect.
   uint8_t expectedOpCode = ObexResponseCode::Success;
   if (mLastCommand == ObexRequestCode::Put) {
     expectedOpCode = ObexResponseCode::Continue;
@@ -1057,16 +1076,23 @@ BluetoothOppManager::ClientDataHandler(U
         PostDelayedTask(FROM_HERE, new CloseSocketTask(mSocket), 1000);
     }
   } else if (mLastCommand == ObexRequestCode::Connect) {
     MOZ_ASSERT(!mFileName.IsEmpty());
     MOZ_ASSERT(mBlob);
 
     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];
     mRemoteMaxPacketLength = (static_cast<int>(data[5]) << 8) | data[6];
 
     // The length of file name exceeds maximum length.
     int fileNameByteLen = (mFileName.Length() + 1) * 2;
     int headerLen = kPutRequestHeaderSize + kPutRequestAppendHeaderSize;