Bug 932496 - Add length check to prevent from memory pollusion, r=gyeh
authorEric Chou <echou@mozilla.com>
Fri, 06 Dec 2013 16:23:35 +0800
changeset 173895 0f2650f8c995ebcc116d42b40157159365d8ce0c
parent 173894 075711113a28e80d32e25060a4ec8ad4e7244106
child 173896 0ae69adce1285f044e1d6475c710142bc8d6103c
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgyeh
bugs932496
milestone28.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 932496 - Add length check to prevent from memory pollusion, r=gyeh
dom/bluetooth/ObexBase.cpp
dom/bluetooth/ObexBase.h
dom/bluetooth/bluedroid/BluetoothOppManager.cpp
dom/bluetooth/bluez/BluetoothOppManager.cpp
--- a/dom/bluetooth/ObexBase.cpp
+++ b/dom/bluetooth/ObexBase.cpp
@@ -73,52 +73,62 @@ AppendHeaderConnectionId(uint8_t* aRetBu
 void
 SetObexPacketInfo(uint8_t* aRetBuf, uint8_t aOpcode, int aPacketLength)
 {
   aRetBuf[0] = aOpcode;
   aRetBuf[1] = (aPacketLength & 0xFF00) >> 8;
   aRetBuf[2] = aPacketLength & 0x00FF;
 }
 
-void
+bool
 ParseHeaders(const uint8_t* aHeaderStart,
              int aTotalLength,
              ObexHeaderSet* aRetHandlerSet)
 {
   const uint8_t* ptr = aHeaderStart;
 
   while (ptr - aHeaderStart < aTotalLength) {
     ObexHeaderId headerId = (ObexHeaderId)*ptr++;
 
-    int contentLength = 0;
+    uint16_t contentLength = 0;
     uint8_t highByte, lowByte;
 
     // Defined in 2.1 OBEX Headers, IrOBEX 1.2
     switch (headerId >> 6)
     {
       case 0x00:
         // Null-terminated Unicode text, length prefixed with 2-byte
         // unsigned integer.
       case 0x01:
         // byte sequence, length prefixed with 2 byte unsigned integer.
         highByte = *ptr++;
         lowByte = *ptr++;
-        contentLength = (((int)highByte << 8) | lowByte) - 3;
+        contentLength = (((uint16_t)highByte << 8) | lowByte) - 3;
         break;
 
       case 0x02:
         // 1 byte quantity
         contentLength = 1;
         break;
 
       case 0x03:
         // 4 byte quantity
         contentLength = 4;
         break;
     }
 
+    // Length check to prevent from memory pollusion.
+    if (ptr + contentLength > aHeaderStart + aTotalLength) {
+      // Severe error occurred. We can't even believe the received data, so
+      // clear all headers.
+      MOZ_ASSERT(false);
+      aRetHandlerSet->ClearHeaders();
+      return false;
+    }
+
     aRetHandlerSet->AddHeader(new ObexHeader(headerId, contentLength, ptr));
-
     ptr += contentLength;
   }
+
+  return true;
 }
 
 END_BLUETOOTH_NAMESPACE
--- a/dom/bluetooth/ObexBase.h
+++ b/dom/bluetooth/ObexBase.h
@@ -97,17 +97,18 @@ enum ObexResponseCode {
   ServiceUnavailable = 0xD3,
   GatewayTimeout = 0xD4,
   HttpVersionNotSupported = 0xD5,
 
   DatabaseFull = 0xE0,
   DatabaseLocked = 0xE1,
 };
 
-class ObexHeader {
+class ObexHeader
+{
 public:
   ObexHeader(ObexHeaderId aId, int aDataLength, const uint8_t* aData)
     : mId(aId)
     , mDataLength(aDataLength)
     , mData(nullptr)
   {
     mData = new uint8_t[mDataLength];
     memcpy(mData, aData, aDataLength);
@@ -117,21 +118,19 @@ public:
   {
   }
 
   ObexHeaderId mId;
   int mDataLength;
   nsAutoArrayPtr<uint8_t> mData;
 };
 
-class ObexHeaderSet {
+class ObexHeaderSet
+{
 public:
-  uint8_t mOpcode;
-  nsTArray<nsAutoPtr<ObexHeader> > mHeaders;
-
   ObexHeaderSet(uint8_t aOpcode) : mOpcode(aOpcode)
   {
   }
 
   ~ObexHeaderSet()
   {
   }
 
@@ -236,23 +235,36 @@ public:
     for (int i = 0; i < length; ++i) {
       if (mHeaders[i]->mId == aId) {
         return true;
       }
     }
 
     return false;
   }
+
+  void ClearHeaders()
+  {
+    mHeaders.Clear();
+  }
+
+private:
+  uint8_t mOpcode;
+  nsTArray<nsAutoPtr<ObexHeader> > mHeaders;
 };
 
 int AppendHeaderName(uint8_t* aRetBuf, const char* aName, int aLength);
 int AppendHeaderBody(uint8_t* aRetBuf, uint8_t* aData, int aLength);
 int AppendHeaderEndOfBody(uint8_t* aRetBuf);
 int AppendHeaderLength(uint8_t* aRetBuf, int aObjectLength);
 int AppendHeaderConnectionId(uint8_t* aRetBuf, int aConnectionId);
 void SetObexPacketInfo(uint8_t* aRetBuf, uint8_t aOpcode, int aPacketLength);
-void ParseHeaders(const uint8_t* aHeaderStart,
+
+/**
+ * @return true when the message was parsed without any error, false otherwise.
+ */
+bool ParseHeaders(const uint8_t* aHeaderStart,
                   int aTotalLength,
                   ObexHeaderSet* aRetHanderSet);
 
 END_BLUETOOTH_NAMESPACE
 
 #endif
--- a/dom/bluetooth/bluedroid/BluetoothOppManager.cpp
+++ b/dom/bluetooth/bluedroid/BluetoothOppManager.cpp
@@ -798,35 +798,41 @@ BluetoothOppManager::ServerDataHandler(U
     }
   }
 
   ObexHeaderSet pktHeaders(opCode);
   if (opCode == ObexRequestCode::Connect) {
     // Section 3.3.1 "Connect", IrOBEX 1.2
     // [opcode:1][length:2][version:1][flags:1][MaxPktSizeWeCanReceive:2]
     // [Headers:var]
-    ParseHeaders(&aMessage->mData[7],
-                 receivedLength - 7,
-                 &pktHeaders);
+    if (!ParseHeaders(&aMessage->mData[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]
-    ParseHeaders(&aMessage->mData[3],
-                receivedLength - 3,
-                &pktHeaders);
+    if (!ParseHeaders(&aMessage->mData[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]
-    ParseHeaders(&aMessage->mData[3],
-                receivedLength - 3,
-                &pktHeaders);
+    if (!ParseHeaders(&aMessage->mData[3], receivedLength - 3, &pktHeaders)) {
+      ReplyError(ObexResponseCode::BadRequest);
+      return;
+    }
+
     ReplyToDisconnectOrAbort();
     AfterOppDisconnected();
     FileTransferComplete();
   } else if (opCode == ObexRequestCode::Put ||
              opCode == ObexRequestCode::PutFinal) {
     if (!ComposePacket(opCode, aMessage)) {
       return;
     }
@@ -1192,17 +1198,17 @@ BluetoothOppManager::ReplyToPut(bool aFi
   }
 
   SendObexData(req, opcode, index);
 }
 
 void
 BluetoothOppManager::ReplyError(uint8_t aError)
 {
-  if (!mConnected) return;
+  BT_LOGR("error: %d", aError);
 
   // Section 3.2 "Response Format", IrOBEX 1.2
   // [opcode:1][length:2][Headers:var]
   uint8_t req[255];
   int index = 3;
 
   SendObexData(req, aError, index);
 }
--- a/dom/bluetooth/bluez/BluetoothOppManager.cpp
+++ b/dom/bluetooth/bluez/BluetoothOppManager.cpp
@@ -814,35 +814,41 @@ BluetoothOppManager::ServerDataHandler(U
     }
   }
 
   ObexHeaderSet pktHeaders(opCode);
   if (opCode == ObexRequestCode::Connect) {
     // Section 3.3.1 "Connect", IrOBEX 1.2
     // [opcode:1][length:2][version:1][flags:1][MaxPktSizeWeCanReceive:2]
     // [Headers:var]
-    ParseHeaders(&aMessage->mData[7],
-                 receivedLength - 7,
-                 &pktHeaders);
+    if (!ParseHeaders(&aMessage->mData[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]
-    ParseHeaders(&aMessage->mData[3],
-                receivedLength - 3,
-                &pktHeaders);
+    if (!ParseHeaders(&aMessage->mData[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]
-    ParseHeaders(&aMessage->mData[3],
-                receivedLength - 3,
-                &pktHeaders);
+    if (!ParseHeaders(&aMessage->mData[3], receivedLength - 3, &pktHeaders)) {
+      ReplyError(ObexResponseCode::BadRequest);
+      return;
+    }
+
     ReplyToDisconnectOrAbort();
     AfterOppDisconnected();
     FileTransferComplete();
   } else if (opCode == ObexRequestCode::Put ||
              opCode == ObexRequestCode::PutFinal) {
     if (!ComposePacket(opCode, aMessage)) {
       return;
     }
@@ -1208,17 +1214,17 @@ BluetoothOppManager::ReplyToPut(bool aFi
   }
 
   SendObexData(req, opcode, index);
 }
 
 void
 BluetoothOppManager::ReplyError(uint8_t aError)
 {
-  if (!mConnected) return;
+  BT_LOGR("error: %d", aError);
 
   // Section 3.2 "Response Format", IrOBEX 1.2
   // [opcode:1][length:2][Headers:var]
   uint8_t req[255];
   int index = 3;
 
   SendObexData(req, aError, index);
 }