Bug 1347748 - Error check the write methods. r=layzell, a=ritu
authorNeil Deakin <neil@mozilla.com>
Wed, 29 Mar 2017 06:32:02 -0400
changeset 578464 145905da25d32c8fd2b1866780a8f0f4f99f5ced
parent 578463 0f6dd3564c7681b5d3da97d52733261308377c83
child 578465 c5012009a0b2203de9e59474b6b68933343bcd00
push id58939
push userbmo:cku@mozilla.com
push dateTue, 16 May 2017 04:17:59 +0000
reviewerslayzell, ritu
bugs1347748
milestone52.1.2
Bug 1347748 - Error check the write methods. r=layzell, a=ritu
dom/events/DataTransfer.cpp
--- a/dom/events/DataTransfer.cpp
+++ b/dom/events/DataTransfer.cpp
@@ -934,17 +934,24 @@ DataTransfer::GetTransferable(uint32_t a
   }
   transferable->Init(aLoadContext);
 
   nsCOMPtr<nsIStorageStream> storageStream;
   nsCOMPtr<nsIBinaryOutputStream> stream;
 
   bool added = false;
   bool handlingCustomFormats = true;
-  uint32_t totalCustomLength = 0;
+
+  // When writing the custom data, we need to ensure that there is sufficient
+  // space for a (uint32_t) data ending type, and the null byte character at
+  // the end of the nsCString. We claim that space upfront and store it in
+  // baseLength. This value will be set to zero if a write error occurs
+  // indicating that the data and length are no longer valid.
+  const uint32_t baseLength = sizeof(uint32_t) + 1;
+  uint32_t totalCustomLength = baseLength;
 
   const char* knownFormats[] = {
     kTextMime, kHTMLMime, kNativeHTMLMime, kRTFMime,
     kURLMime, kURLDataMime, kURLDescriptionMime, kURLPrivateMime,
     kPNGImageMime, kJPEGImageMime, kGIFImageMime, kNativeImageMime,
     kFileMime, kFilePromiseMime, kFilePromiseURLMime,
     kFilePromiseDestFilename, kFilePromiseDirectoryMime,
     kMozTextInternal, kHTMLContext, kHTMLInfo };
@@ -996,18 +1003,19 @@ DataTransfer::GetTransferable(uint32_t a
 
       if (handlingCustomFormats) {
         if (!ConvertFromVariant(variant, getter_AddRefs(convertedData),
                                 &lengthInBytes)) {
           continue;
         }
 
         // When handling custom types, add the data to the stream if this is a
-        // custom type.
-        if (isCustomFormat) {
+        // custom type. If totalCustomLength is 0, then a write error occurred
+        // on a previous item, so ignore any others.
+        if (isCustomFormat && totalCustomLength > 0) {
           // If it isn't a string, just ignore it. The dataTransfer is cached in
           // the drag sesion during drag-and-drop, so non-strings will be
           // available when dragging locally.
           nsCOMPtr<nsISupportsString> str(do_QueryInterface(convertedData));
           if (str) {
             nsAutoString data;
             str->GetData(data);
 
@@ -1017,69 +1025,102 @@ DataTransfer::GetTransferable(uint32_t a
 
               nsCOMPtr<nsIOutputStream> outputStream;
               storageStream->GetOutputStream(0, getter_AddRefs(outputStream));
 
               stream = do_CreateInstance("@mozilla.org/binaryoutputstream;1");
               stream->SetOutputStream(outputStream);
             }
 
-            int32_t formatLength = type.Length() * sizeof(nsString::char_type);
-
-            stream->Write32(eCustomClipboardTypeId_String);
-            stream->Write32(formatLength);
-            stream->WriteBytes((const char *)type.get(),
-                               formatLength);
-            stream->Write32(lengthInBytes);
-            stream->WriteBytes((const char *)data.get(), lengthInBytes);
+            CheckedInt<uint32_t> formatLength =
+              CheckedInt<uint32_t>(type.Length()) * sizeof(nsString::char_type);
 
             // The total size of the stream is the format length, the data
-            // length, two integers to hold the lengths and one integer for the
-            // string flag.
-            totalCustomLength +=
-              formatLength + lengthInBytes + (sizeof(uint32_t) * 3);
+            // length, two integers to hold the lengths and one integer for
+            // the string flag. Guard against large data by ignoring any that
+            // don't fit.
+            CheckedInt<uint32_t> newSize = formatLength + totalCustomLength +
+                                           lengthInBytes + (sizeof(uint32_t) * 3);
+            if (newSize.isValid()) {
+              // If a write error occurs, set totalCustomLength to 0 so that
+              // further processing gets ignored.
+              nsresult rv = stream->Write32(eCustomClipboardTypeId_String);
+              if (NS_WARN_IF(NS_FAILED(rv))) {
+                totalCustomLength = 0;
+                continue;
+              }
+              rv = stream->Write32(formatLength.value());
+              if (NS_WARN_IF(NS_FAILED(rv))) {
+                totalCustomLength = 0;
+                continue;
+              }
+              rv = stream->WriteBytes((const char *)type.get(), formatLength.value());
+              if (NS_WARN_IF(NS_FAILED(rv))) {
+                totalCustomLength = 0;
+                continue;
+              }
+              rv = stream->Write32(lengthInBytes);
+              if (NS_WARN_IF(NS_FAILED(rv))) {
+                totalCustomLength = 0;
+                continue;
+              }
+              rv = stream->WriteBytes((const char *)data.get(), lengthInBytes);
+              if (NS_WARN_IF(NS_FAILED(rv))) {
+                totalCustomLength = 0;
+                continue;
+              }
+
+              totalCustomLength = newSize.value();
+            }
           }
         }
       } else if (isCustomFormat && stream) {
         // This is the second pass of the loop (handlingCustomFormats is false).
         // When encountering the first custom format, append all of the stream
-        // at this position.
-
-        // Write out a terminator.
-        totalCustomLength += sizeof(uint32_t);
-        stream->Write32(eCustomClipboardTypeId_None);
+        // at this position. If totalCustomLength is 0 indicating a write error
+        // occurred, or no data has been added to it, don't output anything,
+        if (totalCustomLength > baseLength) {
+          // Write out an end of data terminator.
+          nsresult rv = stream->Write32(eCustomClipboardTypeId_None);
+          if (NS_SUCCEEDED(rv)) {
+            nsCOMPtr<nsIInputStream> inputStream;
+            storageStream->NewInputStream(0, getter_AddRefs(inputStream));
 
-        nsCOMPtr<nsIInputStream> inputStream;
-        storageStream->NewInputStream(0, getter_AddRefs(inputStream));
+            RefPtr<nsStringBuffer> stringBuffer =
+              nsStringBuffer::Alloc(totalCustomLength);
 
-        RefPtr<nsStringBuffer> stringBuffer =
-          nsStringBuffer::Alloc(totalCustomLength + 1);
+            // Subtract off the null terminator when reading.
+            totalCustomLength--;
 
-        // Read the data from the string and add a null-terminator as ToString
-        // needs it.
-        uint32_t amountRead;
-        inputStream->Read(static_cast<char*>(stringBuffer->Data()),
-                          totalCustomLength, &amountRead);
-        static_cast<char*>(stringBuffer->Data())[amountRead] = 0;
+            // Read the data from the stream and add a null-terminator as
+            // ToString needs it.
+            uint32_t amountRead;
+            rv = inputStream->Read(static_cast<char*>(stringBuffer->Data()),
+                              totalCustomLength, &amountRead);
+            if (NS_SUCCEEDED(rv)) {
+              static_cast<char*>(stringBuffer->Data())[amountRead] = 0;
 
-        nsCString str;
-        stringBuffer->ToString(totalCustomLength, str);
-        nsCOMPtr<nsISupportsCString>
-          strSupports(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID));
-        strSupports->SetData(str);
+              nsCString str;
+              stringBuffer->ToString(totalCustomLength, str);
+              nsCOMPtr<nsISupportsCString>
+                strSupports(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID));
+              strSupports->SetData(str);
 
-        nsresult rv = transferable->SetTransferData(kCustomTypesMime,
-                                                    strSupports,
-                                                    totalCustomLength);
-        if (NS_FAILED(rv)) {
-          return nullptr;
+              nsresult rv = transferable->SetTransferData(kCustomTypesMime,
+                                                          strSupports,
+                                                          totalCustomLength);
+              if (NS_FAILED(rv)) {
+                return nullptr;
+              }
+
+              added = true;
+            }
+          }
         }
 
-        added = true;
-
         // Clear the stream so it doesn't get used again.
         stream = nullptr;
       } else {
         // This is the second pass of the loop and a known type is encountered.
         // Add it as is.
         if (!ConvertFromVariant(variant, getter_AddRefs(convertedData),
                                 &lengthInBytes)) {
           continue;