Fixed|libappfw: Use-after-free during text layout

When TextDrawable was destroyed while it was still laying out long
text content, it was possible that already deleted objects were
accessed because the background task was not stopped.

Now TextDrawable cancels the text layout operation before finishing
its destructor.

doomsday/sdk/libappfw/include/de/framework/fontlinewrapping.h

@@ -65,6 +65,13 @@ class LIBAPPFW_PUBLIC FontLineWrapping : public Lockable, public shell::ILineWra
     void wrapTextToWidth(String const &text, int maxWidth);
     void wrapTextToWidth(String const &text, Font::RichFormat const &format, int maxWidth);
 
+    /**
+     * Cancels the ongoing wrapping operation. This is useful when doing long wrapping
+     * operations in the background. An exception is thrown from the ongoing 
+     * wrapTextToWidth() call.
+     */
+    void cancel();    
+
     bool isEmpty() const;
     String const &text() const;
     shell::WrappedLine line(int index) const;

doomsday/sdk/libappfw/src/fontlinewrapping.cpp

@@ -70,13 +70,21 @@ DENG2_PIMPL_NOREF(FontLineWrapping)
     int indent;                 ///< Current left indentation (in pixels).
     QList<int> prevIndents;
     int tabStop;
+    volatile bool cancelled = false;
+
+    DENG2_ERROR(CancelError);
 
     Instance() : font(0), maxWidth(0), indent(0), tabStop(0) {}
 
     ~Instance()
     {
         clearLines();
     }
+
+    inline void checkCancel() const
+    {
+        if (cancelled) throw CancelError("FontLineWrapping::checkCancel", "Cancelled");
+    }
 
     void clearLines()
     {
@@ -100,6 +108,7 @@ DENG2_PIMPL_NOREF(FontLineWrapping)
 
     int rangeAdvanceWidth(Rangei const &range) const
     {
+        checkCancel();
         if (font)
         {
             return font->advanceWidth(rangeText(range), format.subRange(range));
@@ -147,6 +156,8 @@ DENG2_PIMPL_NOREF(FontLineWrapping)
      */
     Line *makeLine(Rangei const &range, int width = -1)
     {
+        checkCancel();
+
         if (width < 0)
         {
             // Determine the full width now.
@@ -308,6 +319,8 @@ DENG2_PIMPL_NOREF(FontLineWrapping)
         Lines wrappedLines;
         while (begin < rangeToWrap.end)
         {
+            checkCancel();
+
             int mw = maxWidth;
             if (!wrappedLines.isEmpty() && subsequentMaxWidth > 0) mw = subsequentMaxWidth;
 
@@ -539,6 +552,7 @@ void FontLineWrapping::reset()
     d->indent = 0;
     d->prevIndents.clear();
     d->tabStop = 0;
+    d->cancelled = false;
 }
 
 void FontLineWrapping::wrapTextToWidth(String const &text, int maxWidth)
@@ -549,7 +563,7 @@ void FontLineWrapping::wrapTextToWidth(String const &text, int maxWidth)
 void FontLineWrapping::wrapTextToWidth(String const &text, Font::RichFormat const &format, int maxWidth)
 {
     DENG2_GUARD(this);
-
+    
     String newText = text;
 
     clear();
@@ -618,6 +632,11 @@ void FontLineWrapping::wrapTextToWidth(String const &text, Font::RichFormat cons
     }
 #endif
 }
+
+void FontLineWrapping::cancel()
+{
+    d->cancelled = true;
+}
 
 String const &FontLineWrapping::text() const
 {

doomsday/sdk/libappfw/src/textdrawable.cpp

@@ -13,7 +13,7 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
  * General Public License for more details. You should have received a copy of
  * the GNU Lesser General Public License along with this program; if not, see:
- * http://www.gnu.org/licenses</small> 
+ * http://www.gnu.org/licenses</small>
  */
 
 #include "de/TextDrawable"
@@ -132,11 +132,24 @@ DENG2_PIMPL(TextDrawable)
             , _font(font)
             , _style(style)
             , _valid(inst->sync)
+            , _wrapper(nullptr)
         {
             d->audienceForDeletion += this;
         }
-
+        
         void runTask()
+        {
+            try
+            {
+                runWrapTask();
+            }
+            catch (Error const &)
+            {
+                // Cancelled.
+            }
+        }
+
+        void runWrapTask()
         {
             // Check that it's okay if we start the operation now.
             {
@@ -151,36 +164,32 @@ DENG2_PIMPL(TextDrawable)
             }
 
             // Ok, we have a go. Set up the wrapper first.
-            auto *wrapper = new Wrapper;
-            wrapper->setFont(_font);
+            _wrapper.reset(new Wrapper);
+            _wrapper->setFont(_font);
             if (_style)
             {
-                wrapper->format.setStyle(*_style);
+                _wrapper->format.setStyle(*_style);
             }
-            wrapper->plainText = wrapper->format.initFromStyledText(_text);
-            
+            _wrapper->plainText = _wrapper->format.initFromStyledText(_text);
+
             // This is where most of the time will be spent:
-            wrapper->wrapTextToWidth(wrapper->plainText, wrapper->format, _width);
+            _wrapper->wrapTextToWidth(_wrapper->plainText, _wrapper->format, _width);
 
             // Pass the finished wrapping to the owner.
             {
                 DENG2_GUARD(d);
                 if (d) d->audienceForDeletion -= this;
                 if (d && d->sync.isValid(_valid))
                 {
-                    d->incoming.reset(wrapper);
-                }
-                else
-                {
-                    // Well, that was a waste of time.
-                    delete wrapper;
+                    d->incoming.reset(_wrapper.release());
                 }
             }
         }
 
         void ownerDeleted()
         {
             d = nullptr;
+            if (_wrapper) _wrapper->cancel();
         }
 
     private:
@@ -190,6 +199,7 @@ DENG2_PIMPL(TextDrawable)
         Font const &_font;
         Font::RichFormat::IStyle const *_style;
         duint32 _valid;
+        std::unique_ptr<Wrapper> _wrapper;
     };
 
     bool inited { false };
@@ -217,6 +227,8 @@ DENG2_PIMPL(TextDrawable)
         // Let the background tasks know that we are gone.
         DENG2_FOR_AUDIENCE(Deletion, i) i->ownerDeleted();
 
+        tasks.waitForDone();
+
         delete visibleWrap;
     }