From e112c2ee20f89f288a4f0a13827f0b64067096f0 Mon Sep 17 00:00:00 2001 From: Kevin Funk Date: Fri, 27 Jun 2014 13:49:55 +0200 Subject: [PATCH] Make QExplicitlySharedDataPointer copy-ctor from QESDP more safe With "QExplicitlySharedDataPointer::QExplicitlySharedDataPointer( const QExplicitlySharedDataPointer & other)" implicitly doing an static_cast(...) on other.data(), this could lead to dangerous use of this copy constructor. Example code: QExplicitlySharedDataPointer base(new Base); QExplicitlySharedDataPointer derived(base); // that works! This patchs disables the use of the static_cast, and adds a new define called QT_ENABLE_QEXPLICITLYSHAREDDATAPOINTER_STATICCAST to re-enable that code path. Note, that the other way-around (assigning 'derived' to 'base') still works as intended. Other side note: QtXmlPatterns is relying heavily on the hidden static_cast "feature". The other default Qt modules compile fine with the static_cast removed. [ChangeLog][Important Behavior Changes] QExplicitelySharedDataPointer's copy constructor which performs a static_cast from "X *" to "T *" (when constructing a QExplicitlySharedDataPointer from a QExplicitlySharedDataPointer) doesn't perform a static_cast from "X *" to "T *" any more. Instead, an implicit cast is now performed. This change will break compilation of code that relied on the downcast (i.e. cast towards a more derived type) of the templated type when copy costructing a QExplicitelySharedDataPointer object. Please refer to the class documentation for more information about this issue and a workaround to keep old code compiling. Change-Id: Id32aba6cda4e6d44728d7bc3a5c0c7a20f19adc6 Reviewed-by: Kevin Funk Reviewed-by: Thiago Macieira --- src/corelib/tools/qshareddata.cpp | 32 +++++++++++++++++++++++++++---- src/corelib/tools/qshareddata.h | 7 ++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/corelib/tools/qshareddata.cpp b/src/corelib/tools/qshareddata.cpp index 941e58ee7c..b5eed0f504 100644 --- a/src/corelib/tools/qshareddata.cpp +++ b/src/corelib/tools/qshareddata.cpp @@ -538,10 +538,34 @@ QT_BEGIN_NAMESPACE /*! \fn QExplicitlySharedDataPointer::QExplicitlySharedDataPointer(const QExplicitlySharedDataPointer& other) This copy constructor is different in that it allows \a other to be a different type of explicitly shared data pointer but one that has - a compatible shared data object. It performs a static cast of the - \e{d pointer} in \a other and sets the \e {d pointer} of \e this to - the converted \e{d pointer}. It increments the reference count of - the shared data object. + a compatible shared data object. + + By default, the \e{d pointer} of \a other (of type \c{X *}) gets + implicitly converted to the type \c{T *}; the result of this + conversion is set as the \e{d pointer} of \e{this}, and the + reference count of the shared data object is incremented. + + However, if the macro + \c{QT_ENABLE_QEXPLICITLYSHAREDDATAPOINTER_STATICCAST} is defined + before including the \c{QExplicitlySharedDataPointer} header, then + the \e{d pointer} of \a other undergoes a \c{static_cast} to the + type \c{T *}. The result of the cast is then set as the + \e{d pointer} of \e{this}, and the reference count of the shared data + object is incremented. + + \warning relying on such \c{static_cast} is potentially dangerous, + because it allows code like this to compile: + + \code + QExplicitlySharedDataPointer base(new Base); + QExplicitlySharedDataPointer derived(base); // !!! DANGER !!! + \endcode + + Starting from Qt 5.4 the cast is disabled by default. It is + possible to enable it back by defining the + \c{QT_ENABLE_QEXPLICITLYSHAREDDATAPOINTER_STATICCAST} macro, and + therefore to allow old code (that relied on this feature) to + compile without modifications. */ /*! \fn QExplicitlySharedDataPointer& QExplicitlySharedDataPointer::operator=(const QExplicitlySharedDataPointer& other) diff --git a/src/corelib/tools/qshareddata.h b/src/corelib/tools/qshareddata.h index 415ea0d6c7..d85184e995 100644 --- a/src/corelib/tools/qshareddata.h +++ b/src/corelib/tools/qshareddata.h @@ -167,7 +167,12 @@ public: inline QExplicitlySharedDataPointer(const QExplicitlySharedDataPointer &o) : d(o.d) { if (d) d->ref.ref(); } template - inline QExplicitlySharedDataPointer(const QExplicitlySharedDataPointer &o) : d(static_cast(o.data())) + inline QExplicitlySharedDataPointer(const QExplicitlySharedDataPointer &o) +#ifdef QT_ENABLE_QEXPLICITLYSHAREDDATAPOINTER_STATICCAST + : d(static_cast(o.data())) +#else + : d(o.data()) +#endif { if(d) d->ref.ref();