Memory overrun on clipboard paste operation

[raspopov]

Notepad++ does not strictly check the data size of incoming clipboard data and relies only on the string zero terminator ( GetClipboardData() ). So if no zero terminator included into clipboard mistakenly or by evil deed Notepad++ can read a much bigger chunk of memory than expect. Showed as garbage symbols after pasted text. This can provoke even a security issues.

Suggested fix: 09fcd05

[donho]

@raspopov

So if no zero terminator included into clipboard mistakenly or by evil deed Notepad++ can read a much bigger chunk of memory than expect. Showed as garbage symbols after pasted text.

Issue template should be used for providing the instructions to reproduce what you describe above.
If the bug cannot be reproduced, there's no way to prove the linked PR fixed the problem.

[raspopov]

What do you meant by cannot be reproduced? It is 100% reproducible since really no size checks in the code. By the way Notepad++ has other places where it gets data from clipboard with appropriate checks but only in the three of above it doesn't.

[mere-human]

So if no zero terminator included into clipboard mistakenly or by evil deed

What is the way for me as a Notepad++ user to do this?

[raspopov]

No program should trust incoming data and use it without sanity checks. It's a common knowledge in the safe programming and I assume Notepad++ follows it too (or should).

Do you really need a working PoC for this simple bug?
Well here is a program creating a such data in the clipboard:

#include <windows.h>
#include <tchar.h>

int main()
{
	const TCHAR test[] = { _T("0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789") }; // 100 symbols
	const SIZE_T len = sizeof( test ) - 1; // Without terminating zero and not even
	if ( OpenClipboard( NULL ) )
	{
		EmptyClipboard();
		if ( HGLOBAL mem = GlobalAlloc( GHND, len ) )
		{
			if ( LPVOID data = GlobalLock( mem ) )
			{
				memcpy( data, test, len );
				GlobalUnlock( mem );
				SetClipboardData( CF_UNICODETEXT, mem );
			}
		}
		CloseClipboard();
	}
}

Compile and run program.
Open Notepad++ and paste to new document.

What you expect to see:
A string of 99 symbols, the last one should be cut.

What you got (garbage symbols varies in length and value):

[donho]

[raspopov]

It's need no reproduction. It's already confirmed in the source code dude.

[raspopov]

Look at this original Notepad++ code in the file PowerEditor/src/Notepad_plus.cpp:

::OpenClipboard(_pPublicInterface->getHSelf());
HANDLE clipboardData = ::GetClipboardData(clipFormat);
::GlobalSize(clipboardData); <=== WHAT IS THIS USELESS CODE LINE?
LPVOID clipboardDataPtr = ::GlobalLock(clipboardData);
if (!clipboardDataPtr) return;

Seems memory overrun issue exist due interrupted work. In fact developer wanted to check the clipboard data size but forgot about it.

[donho]

I have compiled and used your POC but cannot reproduce the issue you shown.
If the bug cannot be reproduced, there's no way to prove that the PR you provided has fixed it.
I think we all agree with such logic.

[raspopov]

The bug still exists in the Notepad++ code, you logic is not a logic. :-)

I found an issue in the real application (not in PoC), got Notepad++ sources, debugged it and confirmed it in the Notepad++ code, it's an unfinished written size check code (see above), fixed it in that place and other similar places, created a Git fork, committed and created a pull request to the original repository. I done all the work you should done long ago, and you rejected it, I just don't understand why.