From 8082c0dc81b50c44a0cf1984cb2c484b007c64a4 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Sun, 18 Sep 2016 16:44:19 +0200
Subject: QBrush: Fix UB (non-virtual dtor) in QBrush::detach()

As the d-pointer, QBrush uses a QScopedPointer with a
custom deleter that checks for QBrushData::style and
casts the QBrushData pointer down to corresponding
subclasses before calling delete on them.

In QBrush::detach(), however, any of the three brush
data classes were held in a QScopedPointer _without_
the custom deleter, invoking UB when that scoped
pointer would ever get to be the one that deleted
the payload instead of handing it over to the objects
d-pointer.

Found by making dtors protected following a Coverity
report wrongly marked as 'dismissed' (these static
checks are not included in this patch, since they
are binary-incompatible), to find out where Coverity
could possibly see a problem.

Also replace the d.reset(x.take()) that allowed this
mismatch to compile with d.swap(x), which nicely
ensures that x and d are of the same type.

Coverity-Id: 11772
Change-Id: I85e2c205df9291bd7508b6c90f7b03fbe8c3bcd2
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/gui/painting/qbrush.cpp | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

(limited to 'src/gui/painting/qbrush.cpp')

diff --git a/src/gui/painting/qbrush.cpp b/src/gui/painting/qbrush.cpp
index 9e039b7ae4..ee42f2b511 100644
--- a/src/gui/painting/qbrush.cpp
+++ b/src/gui/painting/qbrush.cpp
@@ -579,7 +579,7 @@ void QBrush::detach(Qt::BrushStyle newStyle)
     if (newStyle == d->style && d->ref.load() == 1)
         return;
 
-    QScopedPointer<QBrushData> x;
+    QScopedPointer<QBrushData, QBrushDataPointerDeleter> x;
     switch(newStyle) {
     case Qt::TexturePattern: {
         QTexturedBrushData *tbd = new QTexturedBrushData;
@@ -595,28 +595,30 @@ void QBrush::detach(Qt::BrushStyle newStyle)
         }
     case Qt::LinearGradientPattern:
     case Qt::RadialGradientPattern:
-    case Qt::ConicalGradientPattern:
-        x.reset(new QGradientBrushData);
+    case Qt::ConicalGradientPattern: {
+        QGradientBrushData *gbd = new QGradientBrushData;
         switch (d->style) {
         case Qt::LinearGradientPattern:
         case Qt::RadialGradientPattern:
         case Qt::ConicalGradientPattern:
-            static_cast<QGradientBrushData *>(x.data())->gradient =
+            gbd->gradient =
                     static_cast<QGradientBrushData *>(d.data())->gradient;
             break;
         default:
             break;
         }
+        x.reset(gbd);
         break;
+        }
     default:
         x.reset(new QBrushData);
         break;
     }
-    x->ref.store(1);
+    x->ref.store(1); // must be first lest the QBrushDataPointerDeleter turns into a no-op
     x->style = newStyle;
     x->color = d->color;
     x->transform = d->transform;
-    d.reset(x.take());
+    d.swap(x);
 }
 
 
-- 
cgit v1.2.3