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

diff --git a/src/msw/ole/dataobj.cpp b/src/msw/ole/dataobj.cpp
index bd369ee4e113e645ad9694ac982825d2c62db84d..540d2330bf565266690ac45022f04d8e4b8ab0ee 100644 (file)
--- 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<wxTextDataObject *>(m_dataObjectLast)->GetText();
+    }
 
     return url;
 }

diff --git a/tests/misc/guifuncs.cpp b/tests/misc/guifuncs.cpp
index ec2155b3d0cb00d0580ce654a8d905fa9b50ff51..184675051d72e9d35a580d8721caa2bef71de8b1 100644 (file)
--- 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();
+}
+