From 791f7574a08f7dacdb006b5788ae738a4134ca85 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 30 Aug 2009 17:25:42 +0000 Subject: [PATCH] Fix buffer overflow in wxURLDataObject. The code in CFSTR_SHELLURLDataObject::GetDataHere() was confused by ANSI/Unicode and ended up overwriting output buffer because of it. Moreover, this function was actually completely unnecessary as the base class version did work correctly. Closes #11102 (thanks to Tim Kosse). git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@61788 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- src/msw/ole/dataobj.cpp | 40 +++++++++++++++++++++------------------- tests/misc/guifuncs.cpp | 17 +++++++++++++++++ 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/msw/ole/dataobj.cpp b/src/msw/ole/dataobj.cpp index bd369ee4e1..540d2330bf 100644 --- a/src/msw/ole/dataobj.cpp +++ b/src/msw/ole/dataobj.cpp @@ -1182,23 +1182,6 @@ public: return buffer; } -#if wxUSE_UNICODE - virtual bool GetDataHere( void* buffer ) const - { - // CFSTR_SHELLURL is _always_ ANSI! - wxCharBuffer char_buffer( GetDataSize() ); - wxCustomDataObject::GetDataHere( (void*)char_buffer.data() ); - wxString unicode_buffer( char_buffer, wxConvLibc ); - memcpy( buffer, unicode_buffer.c_str(), - ( unicode_buffer.length() + 1 ) * sizeof(wxChar) ); - - return true; - } - virtual bool GetDataHere(const wxDataFormat& WXUNUSED(format), - void *buf) const - { return GetDataHere(buf); } -#endif - wxDECLARE_NO_COPY_CLASS(CFSTR_SHELLURLDataObject); }; @@ -1236,9 +1219,28 @@ wxString wxURLDataObject::GetURL() const wxString url; wxCHECK_MSG( m_dataObjectLast, url, wxT("no data in wxURLDataObject") ); - size_t len = m_dataObjectLast->GetDataSize(); + if ( m_dataObjectLast->GetPreferredFormat() == CFSTR_SHELLURL ) + { + const size_t len = m_dataObjectLast->GetDataSize(); + if ( !len ) + return wxString(); - m_dataObjectLast->GetDataHere(wxStringBuffer(url, len)); + // CFSTR_SHELLURL is always ANSI so we need to convert it from it in + // Unicode build +#if wxUSE_UNICODE + wxCharBuffer buf(len); + + if ( m_dataObjectLast->GetDataHere(buf.data()) ) + url = buf; +#else // !wxUSE_UNICODE + // in ANSI build no conversion is necessary + m_dataObjectLast->GetDataHere(wxStringBuffer(url, len)); +#endif // wxUSE_UNICODE/!wxUSE_UNICODE + } + else // must be wxTextDataObject + { + url = static_cast(m_dataObjectLast)->GetText(); + } return url; } diff --git a/tests/misc/guifuncs.cpp b/tests/misc/guifuncs.cpp index ec2155b3d0..184675051d 100644 --- a/tests/misc/guifuncs.cpp +++ b/tests/misc/guifuncs.cpp @@ -22,6 +22,8 @@ #endif // !PCH #include "wx/defs.h" +#include "wx/clipbrd.h" +#include "wx/dataobj.h" // ---------------------------------------------------------------------------- // test class @@ -35,9 +37,11 @@ public: private: CPPUNIT_TEST_SUITE( MiscGUIFuncsTestCase ); CPPUNIT_TEST( DisplaySize ); + CPPUNIT_TEST( URLDataObject ); CPPUNIT_TEST_SUITE_END(); void DisplaySize(); + void URLDataObject(); DECLARE_NO_COPY_CLASS(MiscGUIFuncsTestCase) }; @@ -71,3 +75,16 @@ void MiscGUIFuncsTestCase::DisplaySize() CPPUNIT_ASSERT( sz.x < 1000 && sz.y < 1000 ); } +void MiscGUIFuncsTestCase::URLDataObject() +{ + // this tests for buffer overflow, see #11102 + const char * const + url = "http://something.long.to.overwrite.plenty.memory.example.com"; + wxURLDataObject * const dobj = new wxURLDataObject(url); + CPPUNIT_ASSERT_EQUAL( url, dobj->GetURL() ); + + wxClipboardLocker lockClip; + CPPUNIT_ASSERT( wxTheClipboard->SetData(dobj) ); + wxTheClipboard->Flush(); +} + -- 2.45.2