Closed Bug 1356636 Opened 7 years ago Closed 5 years ago

Security bug caused by lack of validation of clipboard data length in nsClipboard::GetNativeDataOffClipboard()

Categories

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

52 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1501482

People

(Reporter: q1, Unassigned)

References

Details

(Keywords: csectype-bounds, sec-moderate, Whiteboard: tpi:+)

Attachments

(2 files)

Attached file bug_833_poc_1.htm
When handling image data (CF_DIBV5), nsClipboard::GetNativeDataOffClipboard() (widget\windows\nsClipboard.cpp) does not pass the actual length of the clipboard data to the handler function nsImageFromClipboard ::GetEncodedImageStream() (widget\windows\nsImageClipboard.cpp).

This omission means that GetEncodedImageStream() cannot determine whether the clipboard data it has been passed is actually as long as it expects. This failure, in turn, can cause GetEncodedImageStream() to read and/or write beyond bounds.

The bug is in line 479:

409: nsresult nsClipboard::GetNativeDataOffClipboard(IDataObject * aDataObject, UINT aIndex, UINT aFormat, const char * aMIMEImageFormat, void ** aData, uint32_t * aLen)
...
470:             case CF_DIBV5:
471:               if (aMIMEImageFormat)
472:               {
473:                 uint32_t allocLen = 0;
474:                 unsigned char * clipboardData;
475:                 if (NS_SUCCEEDED(GetGlobalData(stm.hGlobal, (void **)&clipboardData, &allocLen)))
476:                 {
477:                   nsImageFromClipboard converter;
478:                   nsIInputStream * inputStream;
479:                   converter.GetEncodedImageStream(clipboardData, aMIMEImageFormat, &inputStream);   // addrefs for us, don't release
480:                   if ( inputStream ) {
481:                     *aData = inputStream;
482:                     *aLen = sizeof(nsIInputStream*);
483:                     result = NS_OK;
484:                   }
485:                 }
486:               } break;

This bug is related to several other bugs in clipboard handling, which are described together at https://bugzilla.mozilla.org/show_bug.cgi?id=1353927 .

Attached is a POC that exploits this bug to cause GetEncodedImageStream() to read beyond bounds. When combined with other bugs (see https://bugzilla.mozilla.org/show_bug.cgi?id=1353927 ), this bug can also cause writes beyond bounds.

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 471, 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 fires, step through line 475 and notice that the actual length of the clipboard data (|allocLen|) is 4. Now trace into nsImageFromClipboard ::GetEncodedImageStream() and watch that function read |bmiHeader.biWidth| (offset 4, length 4) and |bmiHeader.biHeight| (offset 8, length 4) from beyond the buffer's end.

The POC has been tested on Win7 SP1 x64, but probably works on all Windows versions.
Attached file bug_833_poc_1.cpp
Blocks: 1353927
Depends on: 1356638
Group: core-security → dom-core-security
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Priority: -- → P4
Whiteboard: tpi:+
Group: dom-core-security → layout-core-security
Group: layout-core-security → core-security-release

This was fixed by bug 1501482. The imgTools::DecodeImageFromBuffer requires a buffer length and does not consume data beyond it.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: