From 0ee2d9be1f8ab706a193e4f0cf095ee79e8210a8 Mon Sep 17 00:00:00 2001
From: Simon Hausmann <simon.hausmann@qt.io>
Date: Fri, 27 Apr 2018 15:47:52 +0200
Subject: Fix heap-use-after-free

Commit a1e5364b492610adf0636fefa3fc400558e211b6 introduced the use of
AST elements at qml compilation unit generation time, which uncovered
the issue that for scripts imported from qml files, the memory pool for
the AST was local to QV4::Script::precompile. Therefore the memory where
the AST stored was freed afterwards and any use after ::precompile()
would produce ASAN errors.

There's no good reason for Script::precompile to have its own local
memory pool.

Change-Id: I4f8eb5ee4e9d62d8874241bc95fc71a912e26cea
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
---
 src/qml/jsruntime/qv4script.cpp | 11 ++++-------
 src/qml/jsruntime/qv4script_p.h |  9 ++++++---
 src/qml/qml/qqmltypeloader.cpp  |  5 +++--
 3 files changed, 13 insertions(+), 12 deletions(-)

(limited to 'src/qml')

diff --git a/src/qml/jsruntime/qv4script.cpp b/src/qml/jsruntime/qv4script.cpp
index daff1c659a..ca6e4c50b1 100644
--- a/src/qml/jsruntime/qv4script.cpp
+++ b/src/qml/jsruntime/qv4script.cpp
@@ -170,19 +170,16 @@ Function *Script::function()
     return vmFunction;
 }
 
-QQmlRefPointer<QV4::CompiledData::CompilationUnit> Script::precompile(QV4::Compiler::Module *module, Compiler::JSUnitGenerator *unitGenerator,
+QQmlRefPointer<QV4::CompiledData::CompilationUnit> Script::precompile(QV4::Compiler::Module *module, QQmlJS::Engine *jsEngine, Compiler::JSUnitGenerator *unitGenerator,
                                                                       const QString &fileName, const QString &finalUrl, const QString &source,
-                                                                      QList<QQmlError> *reportedErrors, Directives *directivesCollector)
+                                                                      QList<QQmlError> *reportedErrors)
 {
     using namespace QV4::Compiler;
     using namespace QQmlJS::AST;
 
-    Engine ee;
-    if (directivesCollector)
-        ee.setDirectives(directivesCollector);
-    Lexer lexer(&ee);
+    Lexer lexer(jsEngine);
     lexer.setCode(source, /*line*/1, /*qml mode*/false);
-    Parser parser(&ee);
+    Parser parser(jsEngine);
 
     parser.parseProgram();
 
diff --git a/src/qml/jsruntime/qv4script_p.h b/src/qml/jsruntime/qv4script_p.h
index 0d1bce6f06..e7189664e2 100644
--- a/src/qml/jsruntime/qv4script_p.h
+++ b/src/qml/jsruntime/qv4script_p.h
@@ -62,6 +62,10 @@ QT_BEGIN_NAMESPACE
 
 class QQmlContextData;
 
+namespace QQmlJS {
+class Engine;
+}
+
 namespace QV4 {
 
 struct Q_QML_EXPORT Script {
@@ -97,10 +101,9 @@ struct Q_QML_EXPORT Script {
 
     Function *function();
 
-    static QQmlRefPointer<CompiledData::CompilationUnit> precompile(
-            QV4::Compiler::Module *module, Compiler::JSUnitGenerator *unitGenerator,
+    static QQmlRefPointer<CompiledData::CompilationUnit> precompile(QV4::Compiler::Module *module, QQmlJS::Engine *jsEngine, Compiler::JSUnitGenerator *unitGenerator,
             const QString &fileName, const QString &finalUrl, const QString &source,
-            QList<QQmlError> *reportedErrors = nullptr, QQmlJS::Directives *directivesCollector = nullptr);
+            QList<QQmlError> *reportedErrors = nullptr);
     static Script *createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl, QString *error);
 
     static ReturnedValue evaluate(ExecutionEngine *engine, const QString &script, QmlContext *qmlContext);
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 2691a15885..29ca10714f 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -2980,11 +2980,12 @@ void QQmlScriptBlob::dataReceived(const SourceCodeData &data)
     }
 
     QmlIR::ScriptDirectivesCollector collector(&irUnit.jsParserEngine, &irUnit.jsGenerator);
+    irUnit.jsParserEngine.setDirectives(&collector);
 
     QList<QQmlError> errors;
     QQmlRefPointer<QV4::CompiledData::CompilationUnit> unit = QV4::Script::precompile(
-                &irUnit.jsModule, &irUnit.jsGenerator, urlString(), finalUrlString(),
-                source, &errors, &collector);
+                &irUnit.jsModule, &irUnit.jsParserEngine, &irUnit.jsGenerator, urlString(), finalUrlString(),
+                source, &errors);
     // No need to addref on unit, it's initial refcount is 1
     source.clear();
     if (!errors.isEmpty()) {
-- 
cgit v1.2.3