QtQuick: fix use-after-free of shader property connections

A use-after-free would occur if the sender of a connection would
disconnect (and destroy the slot object), and then the receiver would
try to clean-up and access the slot object again. The fix is to have
the receiver take out a reference to the slot object, because it will
manage the life-time, and thus delete the slot object when it doesn't
need it anymore.

Change-Id: Ie2033cfb7212acceb2c2cd0bd9e7e45c2dd5e434
Reviewed-by: Robin Burchell <robin.burchell@viroteck.net>

3 files changed, 22 insertions, 3 deletions

diff --git a/src/particles/qquickcustomparticle.cpp b/src/particles/qquickcustomparticle.cpp
index 8ee431aeb2..c08ae3d9ff 100644
--- a/src/particles/qquickcustomparticle.cpp
+++ b/src/particles/qquickcustomparticle.cpp
@@ -207,7 +207,7 @@ void QQuickCustomParticle::updateVertexShader()
 {
     m_common.disconnectPropertySignals(this, Key::VertexShader);
     m_common.uniformData[Key::VertexShader].clear();
-    m_common.signalMappers[Key::VertexShader].clear();
+    m_common.clearSignalMappers(Key::VertexShader);
     m_common.attributes.clear();
     m_common.attributes.append("qt_ParticlePos");
     m_common.attributes.append("qt_ParticleTex");
diff --git a/src/quick/items/qquickopenglshadereffect.cpp b/src/quick/items/qquickopenglshadereffect.cpp
index 3f057ecd64..9d24a6c511 100644
--- a/src/quick/items/qquickopenglshadereffect.cpp
+++ b/src/quick/items/qquickopenglshadereffect.cpp
@@ -187,7 +187,7 @@ public:
 
     explicit MappedSlotObject(PropChangedFunc func)
         : QSlotObjectBase(&impl), _signalIndex(-1), func(func)
-    {}
+    { ref(); }
 
     void setSignalIndex(int idx) { _signalIndex = idx; }
     int signalIndex() const { return _signalIndex; }
@@ -215,6 +215,12 @@ private:
 };
 }
 
+QQuickOpenGLShaderEffectCommon::~QQuickOpenGLShaderEffectCommon()
+{
+    for (int shaderType = 0; shaderType < Key::ShaderTypeCount; ++shaderType)
+        clearSignalMappers(shaderType);
+}
+
 void QQuickOpenGLShaderEffectCommon::disconnectPropertySignals(QQuickItem *item, Key::ShaderType shaderType)
 {
     for (int i = 0; i < uniformData[shaderType].size(); ++i) {
@@ -363,7 +369,7 @@ void QQuickOpenGLShaderEffectCommon::updateShader(QQuickItem *item,
 {
     disconnectPropertySignals(item, shaderType);
     uniformData[shaderType].clear();
-    signalMappers[shaderType].clear();
+    clearSignalMappers(shaderType);
     if (shaderType == Key::VertexShader)
         attributes.clear();
 
@@ -593,6 +599,15 @@ void QQuickOpenGLShaderEffectCommon::propertyChanged(QQuickItem *item,
     }
 }
 
+void QQuickOpenGLShaderEffectCommon::clearSignalMappers(int shader)
+{
+    for (auto mapper : qAsConst(signalMappers[shader])) {
+        if (mapper)
+            mapper->destroyIfLastRef();
+    }
+    signalMappers[shader].clear();
+}
+
 QQuickOpenGLShaderEffect::QQuickOpenGLShaderEffect(QQuickShaderEffect *item, QObject *parent)
     : QObject(parent)
     , m_item(item)
diff --git a/src/quick/items/qquickopenglshadereffect_p.h b/src/quick/items/qquickopenglshadereffect_p.h
index 44b60c97d9..ed56a76409 100644
--- a/src/quick/items/qquickopenglshadereffect_p.h
+++ b/src/quick/items/qquickopenglshadereffect_p.h
@@ -84,6 +84,8 @@ struct Q_QUICK_PRIVATE_EXPORT QQuickOpenGLShaderEffectCommon
         : host(host), mappedPropertyChanged(mappedPropertyChanged), fileSelector(nullptr)
     { }
 
+    ~QQuickOpenGLShaderEffectCommon();
+
     void disconnectPropertySignals(QQuickItem *item, Key::ShaderType shaderType);
     void connectPropertySignals(QQuickItem *item, const QMetaObject *itemMetaObject, Key::ShaderType shaderType);
     void updateParseLog(bool ignoreAttributes);
@@ -97,6 +99,8 @@ struct Q_QUICK_PRIVATE_EXPORT QQuickOpenGLShaderEffectCommon
     void sourceDestroyed(QObject *object);
     void propertyChanged(QQuickItem *item, const QMetaObject *itemMetaObject, int mappedId, bool *textureProviderChanged);
 
+    void clearSignalMappers(int shader);
+
     QObject *host;
     std::function<void(int)> mappedPropertyChanged;
     Key source;