From b6cf0672e6a4724e912329d205c2aecd53ef5053 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 29 Aug 2022 12:00:52 +0200 Subject: [PATCH] Allow limited extensions to globals We can allow a, overriding data members of globals, such as Error.name b, adding members that don't clash with any internals c, any manipulation of toString(), toLocaleString(), valueOf(), and constructor To that effect, add a "Locked" flag to our internal classes. If that is set, disallow changing prototypes and when defining a property, check if it shadows any non-configurable property. Furthermore, make all non-primitive properties that are not meant to be overridden non-configurable and non-writable. constructor, toString(), toLocaleString() and valueOf() are exempt because they are explicitly meant to be overridden by users. Therefore, we let that happen and refrain from optimizing them or triggering their implicit invocation in optimized code. [ChangeLog][QtQml][Important Behavior Changes] The JavaScript global objects are not frozen anymore in a QML engine. Instead, they are selectively locked. You can extend the objects with new members as long as you don't shadow any existing methods, and you can change or override data members. This also means that most methods of Object.prototype, which was previously exempt from the freezing, cannot be changed anymore. You can, however, change or override constructor, toString(), toLocaleString() and valueOf() on any prototype. Those are clearly meant to be overridden by user code. Fixes: QTBUG-101298 Task-number: QTBUG-84341 Change-Id: Id77db971f76c8f48b18e7a93607da5f947ecfc3e Reviewed-by: Fabian Kosmale --- src/qml/jsruntime/qv4engine.cpp | 54 ++++++++- src/qml/jsruntime/qv4engine_p.h | 1 + src/qml/jsruntime/qv4internalclass.cpp | 21 ++++ src/qml/jsruntime/qv4internalclass_p.h | 4 + src/qml/jsruntime/qv4object.cpp | 30 ++++- .../qml/qqmlengine/data/lockedRootObject.qml | 108 ++++++++++++++++++ tests/auto/qml/qqmlengine/tst_qqmlengine.cpp | 22 ++++ 7 files changed, 235 insertions(+), 5 deletions(-) create mode 100644 tests/auto/qml/qqmlengine/data/lockedRootObject.qml diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp index 26677a37b9..2831dfc0af 100644 --- a/src/qml/jsruntime/qv4engine.cpp +++ b/src/qml/jsruntime/qv4engine.cpp @@ -2129,7 +2129,7 @@ QV4::ReturnedValue ExecutionEngine::callInContext(QV4::Function *function, QObje void ExecutionEngine::initQmlGlobalObject() { initializeGlobal(); - freezeObject(*globalObject); + lockObject(*globalObject); } void ExecutionEngine::initializeGlobal() @@ -2233,6 +2233,58 @@ void ExecutionEngine::freezeObject(const QV4::Value &value) freeze_recursive(this, o); } +void ExecutionEngine::lockObject(const QV4::Value &value) +{ + QV4::Scope scope(this); + ScopedObject object(scope, value); + if (!object) + return; + + std::vector stack { object->d() }; + + // Methods meant to be overridden + const PropertyKey writableMembers[] = { + id_toString()->propertyKey(), + id_toLocaleString()->propertyKey(), + id_valueOf()->propertyKey(), + id_constructor()->propertyKey() + }; + const auto writableBegin = std::begin(writableMembers); + const auto writableEnd = std::end(writableMembers); + + while (!stack.empty()) { + object = stack.back(); + stack.pop_back(); + + if (object->as() || object->internalClass()->isLocked()) + continue; + + Scoped locked(scope, object->internalClass()->locked()); + QV4::ScopedObject member(scope); + + // Taking this copy is cheap. It's refcounted. This avoids keeping a reference + // to the original IC. + const SharedInternalClassData nameMap = locked->d()->nameMap; + + for (uint i = 0, end = locked->d()->size; i < end; ++i) { + const PropertyKey key = nameMap.at(i); + if (!key.isStringOrSymbol()) + continue; + if ((member = *object->propertyData(i))) { + stack.push_back(member->d()); + if (std::find(writableBegin, writableEnd, key) == writableEnd) { + PropertyAttributes attributes = locked->d()->find(key).attributes; + attributes.setConfigurable(false); + attributes.setWritable(false); + locked = locked->changeMember(key, attributes); + } + } + } + + object->setInternalClass(locked->d()); + } +} + void ExecutionEngine::startTimer(const QString &timerName) { if (!m_time.isValid()) diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h index c4a966eb07..caa22992af 100644 --- a/src/qml/jsruntime/qv4engine_p.h +++ b/src/qml/jsruntime/qv4engine_p.h @@ -673,6 +673,7 @@ public: void createQtObject(); void freezeObject(const QV4::Value &value); + void lockObject(const QV4::Value &value); // Return the list of illegal id names (the names of the properties on the global object) const QSet &illegalNames() const; diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp index bd47d8efbc..64f10175ea 100644 --- a/src/qml/jsruntime/qv4internalclass.cpp +++ b/src/qml/jsruntime/qv4internalclass.cpp @@ -395,6 +395,9 @@ static Heap::InternalClass *cleanInternalClass(Heap::InternalClass *orig) case InternalClassTransition::Frozen: child = child->d()->frozen(); continue; + case InternalClassTransition::Locked: + child = child->d()->locked(); + continue; default: Q_ASSERT(it->flags != 0); Q_ASSERT(it->flags < InternalClassTransition::StructureChange); @@ -515,6 +518,24 @@ Heap::InternalClass *InternalClass::nonExtensible() return newClass; } +InternalClass *InternalClass::locked() +{ + if (isLocked()) + return this; + + Transition temp = { { PropertyKey::invalid() }, nullptr, Transition::Locked}; + Transition &t = lookupOrInsertTransition(temp); + if (t.lookup) + return t.lookup; + + Heap::InternalClass *newClass = engine->newClass(this); + newClass->flags |= Locked; + + t.lookup = newClass; + Q_ASSERT(t.lookup); + return newClass; +} + void InternalClass::addMember(QV4::Object *object, PropertyKey id, PropertyAttributes data, InternalClassEntry *entry) { Q_ASSERT(id.isStringOrSymbol()); diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h index cd5458c910..df73ee6ccc 100644 --- a/src/qml/jsruntime/qv4internalclass_p.h +++ b/src/qml/jsruntime/qv4internalclass_p.h @@ -282,6 +282,7 @@ struct InternalClassTransition ProtoClass = StructureChange | (1 << 3), Sealed = StructureChange | (1 << 4), Frozen = StructureChange | (1 << 5), + Locked = StructureChange | (1 << 6), }; bool operator==(const InternalClassTransition &other) const @@ -299,6 +300,7 @@ struct InternalClass : Base { Sealed = 1 << 1, Frozen = 1 << 2, UsedAsProto = 1 << 3, + Locked = 1 << 4, }; enum { MaxRedundantTransitions = 255 }; @@ -324,6 +326,7 @@ struct InternalClass : Base { bool isSealed() const { return flags & Sealed; } bool isFrozen() const { return flags & Frozen; } bool isUsedAsProto() const { return flags & UsedAsProto; } + bool isLocked() const { return flags & Locked; } void init(ExecutionEngine *engine); void init(InternalClass *other); @@ -331,6 +334,7 @@ struct InternalClass : Base { Q_QML_PRIVATE_EXPORT QString keyAt(uint index) const; Q_REQUIRED_RESULT InternalClass *nonExtensible(); + Q_REQUIRED_RESULT InternalClass *locked(); static void addMember(QV4::Object *object, PropertyKey id, PropertyAttributes data, InternalClassEntry *entry); Q_REQUIRED_RESULT InternalClass *addMember(PropertyKey identifier, PropertyAttributes data, InternalClassEntry *entry = nullptr); diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp index ef4a40f4f3..a160998242 100644 --- a/src/qml/jsruntime/qv4object.cpp +++ b/src/qml/jsruntime/qv4object.cpp @@ -17,10 +17,14 @@ #include "qv4symbol_p.h" #include "qv4proxy_p.h" +#include + #include using namespace QV4; +Q_LOGGING_CATEGORY(lcJavaScriptGlobals, "qt.qml.js.globals") + DEFINE_OBJECT_VTABLE(Object); void Object::setInternalClass(Heap::InternalClass *ic) @@ -946,12 +950,29 @@ bool Object::virtualDefineOwnProperty(Managed *m, PropertyKey id, const Property return o->internalDefineOwnProperty(scope.engine, index, nullptr, p, attrs); } - auto memberIndex = o->internalClass()->find(id); + Scoped ic(scope, o->internalClass()); + auto memberIndex = ic->d()->find(id); if (!memberIndex.isValid()) { if (!o->isExtensible()) return false; + // If the IC is locked, you're not allowed to shadow any unconfigurable properties. + if (ic->d()->isLocked()) { + while (Heap::Object *prototype = ic->d()->prototype) { + ic = prototype->internalClass; + const auto entry = ic->d()->find(id); + if (entry.isValid()) { + if (entry.attributes.isConfigurable()) + break; + qCWarning(lcJavaScriptGlobals).noquote() + << QStringLiteral("You cannot shadow the locked property " + "'%1' in QML.").arg(id.toQString()); + return false; + } + } + } + Scoped name(scope, id.asStringOrSymbol()); ScopedProperty pd(scope); pd->copy(p, attrs); @@ -985,11 +1006,12 @@ bool Object::virtualSetPrototypeOf(Managed *m, const Object *proto) { Q_ASSERT(m->isObject()); Object *o = static_cast(m); - Heap::Object *current = o->internalClass()->prototype; + Heap::InternalClass *ic = o->internalClass(); + Heap::Object *current = ic->prototype; Heap::Object *protod = proto ? proto->d() : nullptr; if (current == protod) return true; - if (!o->internalClass()->isExtensible()) + if (!ic->isExtensible() || ic->isLocked()) return false; Heap::Object *p = protod; while (p) { @@ -999,7 +1021,7 @@ bool Object::virtualSetPrototypeOf(Managed *m, const Object *proto) break; p = p->prototype(); } - o->setInternalClass(o->internalClass()->changePrototype(protod)); + o->setInternalClass(ic->changePrototype(protod)); return true; } diff --git a/tests/auto/qml/qqmlengine/data/lockedRootObject.qml b/tests/auto/qml/qqmlengine/data/lockedRootObject.qml new file mode 100644 index 0000000000..5a677b62cb --- /dev/null +++ b/tests/auto/qml/qqmlengine/data/lockedRootObject.qml @@ -0,0 +1,108 @@ +import QtQml + +QtObject { + property string myErrorName: { + var e = new Error; + try { + e.name = "MyError1"; + } finally { + return e.name; + } + } + + property string errorName: { + var e = new Error; + try { + Error.prototype.name = "MyError2"; + } finally { + return e.name + } + } + + property int mathMax: { + // Cannot change methods of builtins + try { + Math.max = function(a, b) { return 10 }; + } finally { + return Math.max(3, 4) + } + } + + property int extendGlobal: { + // Can add new methods to globals + try { + Array.prototype.myMethod = function() { return 32 } + } finally { + return (new Array).myMethod() + } + } + + property string prototypeTrick: { + // Cannot change prototypes of locked objects + try { + SyntaxError.prototype.setPrototypeOf({ + toLocaleString : function() { return "not a SyntaxError"} + }); + } finally { + return (new SyntaxError).toLocaleString(); + } + } + + property string shadowMethod1: { + // Can override Object.prototype methods meant to be changed + try { + TypeError.prototype.toLocaleString = function() { return "not a TypeError"}; + } finally { + return (new TypeError).toLocaleString(); + } + } + + property bool shadowMethod2: { + // Cannot override Object.prototype methods not meant to be changed + try { + TypeError.prototype.hasOwnProperty = function() { return true }; + } finally { + return (new TypeError).hasOwnProperty("foobar"); + } + } + + property string changeObjectProto1: { + // Can change Object.prototype methods meant to be changed + try { + Object.prototype.toLocaleString = function() { return "not an Object"}; + } finally { + return (new Object).toLocaleString(); + } + } + + property bool changeObjectProto2: { + // Cannot change Object.prototype methods not meant to be changed + try { + Object.prototype.hasOwnProperty = function() { return true }; + } finally { + return (new Object).hasOwnProperty("foobar"); + } + } + + property string defineProperty1: { + // Can define a property that shadows an existing one meant to be changed + try { + Object.defineProperty(URIError.prototype, "toLocaleString", { + value: function() { return "not a URIError" } + }) + } finally { + return (new URIError).toLocaleString(); + } + } + + property bool defineProperty2: { + // Cannot define a property that shadows an existing one not meant to be changed + try { + Object.defineProperty(URIError.prototype, "hasOwnProperty", { + value: function() { return true } + }) + } finally { + return (new URIError).hasOwnProperty("foobar"); + } + } +} diff --git a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp index 36a030541b..949c85913b 100644 --- a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp +++ b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp @@ -71,6 +71,7 @@ private slots: void qobjectToString(); void qtNamespaceInQtObject(); void nativeModuleImport(); + void lockedRootObject(); public slots: QObject *createAQObjectForOwnershipTest () @@ -1637,6 +1638,27 @@ void tst_qqmlengine::nativeModuleImport() QCOMPARE(o->property("e").toString(), QStringLiteral("TheName")); } +void tst_qqmlengine::lockedRootObject() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("lockedRootObject.qml")); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + QTest::ignoreMessage( + QtWarningMsg, "You cannot shadow the locked property 'hasOwnProperty' in QML."); + QScopedPointer o(c.create()); + QCOMPARE(o->property("myErrorName").toString(), QStringLiteral("MyError1")); + QCOMPARE(o->property("errorName").toString(), QStringLiteral("MyError2")); + QCOMPARE(o->property("mathMax").toInt(), 4); + QCOMPARE(o->property("extendGlobal").toInt(), 32); + QCOMPARE(o->property("prototypeTrick").toString(), QStringLiteral("SyntaxError")); + QCOMPARE(o->property("shadowMethod1").toString(), QStringLiteral("not a TypeError")); + QCOMPARE(o->property("shadowMethod2").toBool(), false); + QCOMPARE(o->property("changeObjectProto1").toString(), QStringLiteral("not an Object")); + QCOMPARE(o->property("changeObjectProto2").toBool(), false); + QCOMPARE(o->property("defineProperty1").toString(), QStringLiteral("not a URIError")); + QCOMPARE(o->property("defineProperty2").toBool(), false); +} + QTEST_MAIN(tst_qqmlengine) #include "tst_qqmlengine.moc"