1356650 - Write beyond bounds caused by lack of validation of clipboard image header (and overflow) in nsImageFromClipboard ::GetEncodedImageStream()

Status: RESOLVED DUPLICATE of bug 1501482

(Core :: Widget: Win32, defect, P4)

Description

[q1]

Attachment: bug_834_poc_1.cpp

nsImageFromClipboard::GetEncodedImageStream() (widget\windows\nsImageClipboard.cpp) does not validate its input clipboard image header. Additionally, it does not check for overflow when computing a buffer size. These bugs can cause a read and/or write beyond bounds.

The bugs are between lines 230 and 235 (lack of validation of |bmiHeader.biWidth| and |bmiHeader.biHeight|), on line 235 (unguarded multiplication that can overflow), and on line 240 (lack of validation of |header->bmiHeader.biSize|):


219: nsresult 
220: nsImageFromClipboard::GetEncodedImageStream (unsigned char * aClipboardData, const char * aMIMEFormat, nsIInputStream** aInputStream )
221: {
222:   NS_ENSURE_ARG_POINTER (aInputStream);
223:   NS_ENSURE_ARG_POINTER (aMIMEFormat);
224:   nsresult rv;
225:   *aInputStream = nullptr;
226: 
227:   // pull the size information out of the BITMAPINFO header and
228:   // initialize the image
229:   BITMAPINFO* header = (BITMAPINFO *) aClipboardData;
230:   int32_t width  = header->bmiHeader.biWidth;
231:   int32_t height = header->bmiHeader.biHeight;
232:   // neg. heights mean the Y axis is inverted and we don't handle that case
233:   NS_ENSURE_TRUE(height > 0, NS_ERROR_FAILURE); 
234: 
235:   unsigned char * rgbData = new unsigned char[width * height * 3 /* RGB */];
236: 
237:   if (rgbData) {
238:     BYTE  * pGlobal = (BYTE *) aClipboardData;
239:     // Convert the clipboard image into RGB packed pixel data
240:     rv = ConvertColorBitMap((unsigned char *) (pGlobal + header->bmiHeader.biSize), header, rgbData);
241:     // if that succeeded, encode the bitmap as aMIMEFormat data. Don't return early or we risk leaking rgbData
242:     if (NS_SUCCEEDED(rv)) {
243:       nsAutoCString encoderCID(NS_LITERAL_CSTRING("@mozilla.org/image/encoder;2?type="));
244: 
245:       // Map image/jpg to image/jpeg (which is how the encoder is registered).
246:       if (strcmp(aMIMEFormat, kJPGImageMime) == 0)
247:         encoderCID.AppendLiteral("image/jpeg");
248:       else
249:         encoderCID.Append(aMIMEFormat);
250:       nsCOMPtr<imgIEncoder> encoder = do_CreateInstance(encoderCID.get(), &rv);
251:       if (NS_SUCCEEDED(rv)){
252:         rv = encoder->InitFromData(rgbData, 0, width, height, 3 * width /* RGB * # pixels in a row */, 
253:                                    imgIEncoder::INPUT_FORMAT_RGB, EmptyString());
254:         if (NS_SUCCEEDED(rv)) {
255:           encoder.forget(aInputStream);
256:         }
257:       }
258:     }
259:     delete [] rgbData;
260:   } 
261:   else 
262:     rv = NS_ERROR_OUT_OF_MEMORY;
263: 
264:   return rv;
265: } // GetImage



Attached is a POC that uses these bugs to cause line 235 to overflow, causing an underallocation and write beyond bounds of an attacker-specified amount of attacker-specified data.

Use the POC web page and code segment by creating a new Win32 (GUI-mode) project in Visual Studio. Replace the About() function with the POC code segment. Build the result. Start FF and attach the debugger to it, setting a BP on line 235, above. Load the POC web page. Now run the POC code and select help/about, then OK. Click on "Paste an image here!" and press ctrl/v. When the BP triggers, view the disassembly window to see the multiplication overflow, causing line 235 to allocate 0x8000 bytes for |rgbData| Then trace downstream into nsImageFromClipboard ::ConvertColorBitmap() and watch it write 0x28000 bytes beyond |rgbData|'s bounds.

Comment 1

[q1]

Attachment: bug_834_poc_1.htm

Comment 2

[q1]

The POC was tested on Win7 SP1 x64, but probably works on all Windows versions.

Comment 3

[q1]

You can also cause the call chain beginning at line 252 (encoder->InitFromData(), above) to read beyond bounds by modifying the POC code to use the arguments:

   GlobalAlloc(... + 0x3000);
   ...
   |bmiHeader.biSizeImage| == 0x3000
   |bmiHeader.biWidth| == 128
   |bmiHeader.biHeight| == 128

With the debugger, you can trace control into nsPNGEncoder::InitFromData() with the bogus image height and width, thence into nsPNGEncoder::StartImageEncode(), and into png_set_IHDR() with the same bogus dimensions.

Eventually control ends up in nsPNGEncoder::AddImageFrame(), which then reads far beyond the end of the (0x3000-byte) |rgbData| buffer that GetEncodedImageStream() passed to it when it called InitFromData(); step line 298 in nsPNGEncoder::AddImageFrame() (image\encoders\png\nsPNGEncoder.cpp) to see this occur:

   202: NS_IMETHODIMP
   203: nsPNGEncoder::AddImageFrame(const uint8_t* aData,
   204:                             uint32_t aLength, // (unused, req'd by JS)
   205:                             uint32_t aWidth,
   206:                             uint32_t aHeight,
   207:                             uint32_t aStride,
   208:                             uint32_t aInputFormat,
   209:                             const nsAString& aFrameOptions)
   210: {
   ...
   294:   } else if (aInputFormat == INPUT_FORMAT_RGB ||
   295:              aInputFormat == INPUT_FORMAT_RGBA) {
   296:     // simple RBG(A), no conversion needed
   297:     for (uint32_t y = 0; y < aHeight; y++) {
   298:       png_write_row(mPNG, (uint8_t*)&aData[y * aStride]);
   299:     }

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Bug 1501482 removed this code.