A null compound index value crashes the Databases process.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Mar 2017 21:13:40 +0000 (21:13 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Mar 2017 21:13:40 +0000 (21:13 +0000)
<rdar://problem/30499831> and https://bugs.webkit.org/show_bug.cgi?id=170000

Reviewed by Alex Christensen.

Source/WebCore:

Test: storage/indexeddb/modern/single-entry-index-invalid-key-crash.html

* bindings/js/IDBBindingUtilities.cpp:
(WebCore::createKeyPathArray): Fix the bug by rejecting arrays with any invalid keys in them.

Add some logging:
* Modules/indexeddb/IDBKeyPath.cpp:
(WebCore::loggingString):
* Modules/indexeddb/IDBKeyPath.h:
* Modules/indexeddb/IDBObjectStore.cpp:
(WebCore::IDBObjectStore::createIndex):
* Modules/indexeddb/shared/IDBIndexInfo.cpp:
(WebCore::IDBIndexInfo::loggingString):

LayoutTests:

* storage/indexeddb/modern/resources/single-entry-index-invalid-key-crash.js: Added.
* storage/indexeddb/modern/single-entry-index-invalid-key-crash-expected.txt: Added.
* storage/indexeddb/modern/single-entry-index-invalid-key-crash-private-expected.txt: Added.
* storage/indexeddb/modern/single-entry-index-invalid-key-crash-private.html: Added.
* storage/indexeddb/modern/single-entry-index-invalid-key-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214375 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/storage/indexeddb/modern/resources/single-entry-index-invalid-key-crash.js [new file with mode: 0644]
LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-private-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-private.html [new file with mode: 0644]
LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp
Source/WebCore/Modules/indexeddb/IDBKeyPath.h
Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp
Source/WebCore/Modules/indexeddb/shared/IDBIndexInfo.cpp
Source/WebCore/bindings/js/IDBBindingUtilities.cpp

index 12c4847..a5b4fa1 100644 (file)
@@ -1,3 +1,16 @@
+2017-03-24  Brady Eidson  <beidson@apple.com>
+
+        A null compound index value crashes the Databases process.
+        <rdar://problem/30499831> and https://bugs.webkit.org/show_bug.cgi?id=170000
+
+        Reviewed by Alex Christensen.
+
+        * storage/indexeddb/modern/resources/single-entry-index-invalid-key-crash.js: Added.
+        * storage/indexeddb/modern/single-entry-index-invalid-key-crash-expected.txt: Added.
+        * storage/indexeddb/modern/single-entry-index-invalid-key-crash-private-expected.txt: Added.
+        * storage/indexeddb/modern/single-entry-index-invalid-key-crash-private.html: Added.
+        * storage/indexeddb/modern/single-entry-index-invalid-key-crash.html: Added.
+
 2017-03-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         Skip svg/animations/animations-paused-when-inserted-in-hidden-document* tests on ios-simulator.
diff --git a/LayoutTests/storage/indexeddb/modern/resources/single-entry-index-invalid-key-crash.js b/LayoutTests/storage/indexeddb/modern/resources/single-entry-index-invalid-key-crash.js
new file mode 100644 (file)
index 0000000..3830f5d
--- /dev/null
@@ -0,0 +1,29 @@
+description("Tests that adding to an object store, with a single-entry Index, where the index key is an array that is not entirely valid... does not crash.");
+
+indexedDBTest(prepareDatabase);
+
+function log(message)
+{
+    debug(message);
+}
+
+function prepareDatabase(event)
+{
+    db = event.target.result;
+    os = db.createObjectStore("friends", { keyPath: "id", autoIncrement: true });
+       idx = os.createIndex("[age+shoeSize]", ["age", "shoeSize"]);
+       os.add({ name: "Mark", age: 29, shoeSize: null });
+
+       idx.openCursor().onsuccess = function(event) {
+           if (event.target.result)
+               log("Index unexpectedly has an entry");
+        else
+               log("Index has no entries");
+    };
+       
+    event.target.transaction.oncomplete = function() {
+        finishJSTest();
+    };
+}
+
\ No newline at end of file
diff --git a/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-expected.txt b/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-expected.txt
new file mode 100644 (file)
index 0000000..873cdb8
--- /dev/null
@@ -0,0 +1,14 @@
+Tests that adding to an object store, with a single-entry Index, where the index key is an array that is not entirely valid... does not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
+
+indexedDB.deleteDatabase(dbname)
+indexedDB.open(dbname)
+Index has no entries
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-private-expected.txt b/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-private-expected.txt
new file mode 100644 (file)
index 0000000..873cdb8
--- /dev/null
@@ -0,0 +1,14 @@
+Tests that adding to an object store, with a single-entry Index, where the index key is an array that is not entirely valid... does not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
+
+indexedDB.deleteDatabase(dbname)
+indexedDB.open(dbname)
+Index has no entries
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-private.html b/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash-private.html
new file mode 100644 (file)
index 0000000..fbf0784
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../resources/shared.js"></script>
+</head>
+<body>
+
+<script src="resources/single-entry-index-invalid-key-crash.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash.html b/LayoutTests/storage/indexeddb/modern/single-entry-index-invalid-key-crash.html
new file mode 100644 (file)
index 0000000..fbf0784
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../resources/shared.js"></script>
+</head>
+<body>
+
+<script src="resources/single-entry-index-invalid-key-crash.js"></script>
+</body>
+</html>
index 6b50a0b..c482cbd 100644 (file)
@@ -1,3 +1,24 @@
+2017-03-24  Brady Eidson  <beidson@apple.com>
+
+        A null compound index value crashes the Databases process.
+        <rdar://problem/30499831> and https://bugs.webkit.org/show_bug.cgi?id=170000
+
+        Reviewed by Alex Christensen.
+
+        Test: storage/indexeddb/modern/single-entry-index-invalid-key-crash.html
+
+        * bindings/js/IDBBindingUtilities.cpp:
+        (WebCore::createKeyPathArray): Fix the bug by rejecting arrays with any invalid keys in them.
+        
+        Add some logging:
+        * Modules/indexeddb/IDBKeyPath.cpp:
+        (WebCore::loggingString):
+        * Modules/indexeddb/IDBKeyPath.h:
+        * Modules/indexeddb/IDBObjectStore.cpp:
+        (WebCore::IDBObjectStore::createIndex):
+        * Modules/indexeddb/shared/IDBIndexInfo.cpp:
+        (WebCore::IDBIndexInfo::loggingString):
+
 2017-03-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r214360.
index f415ac8..e76d4f9 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -30,6 +31,7 @@
 
 #include <wtf/ASCIICType.h>
 #include <wtf/dtoa.h>
+#include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
@@ -221,6 +223,31 @@ IDBKeyPath isolatedCopy(const IDBKeyPath& keyPath)
     return WTF::visit(visitor, keyPath);
 }
 
+#ifndef NDEBUG
+String loggingString(const IDBKeyPath& path)
+{
+    auto visitor = WTF::makeVisitor([](const String& string) {
+        return makeString("< ", string, " >");
+    }, [](const Vector<String>& strings) {
+        if (strings.isEmpty())
+            return String("< >");
+
+        StringBuilder builder;
+        builder.append("< ");
+        for (size_t i = 0; i < strings.size() - 1; ++i) {
+            builder.append(strings[i]);
+            builder.append(", ");
+        }
+        builder.append(strings.last());
+        builder.append(" >");
+
+        return builder.toString();
+    });
+
+    return WTF::visit(visitor, path);
+}
+#endif
+
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)
index 7c3f021..c5a04b3 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -52,6 +53,10 @@ inline std::optional<IDBKeyPath> isolatedCopy(const std::optional<IDBKeyPath>& v
     return isolatedCopy(variant.value());
 }
 
+#ifndef NDEBUG
+String loggingString(const IDBKeyPath&);
+#endif
+
 } // namespace WebCore
 
 #endif
index 6f77a90..592c67d 100644 (file)
@@ -424,7 +424,7 @@ ExceptionOr<Ref<IDBRequest>> IDBObjectStore::clear(ExecState& execState)
 
 ExceptionOr<Ref<IDBIndex>> IDBObjectStore::createIndex(ExecState&, const String& name, IDBKeyPath&& keyPath, const IndexParameters& parameters)
 {
-    LOG(IndexedDB, "IDBObjectStore::createIndex %s", name.utf8().data());
+    LOG(IndexedDB, "IDBObjectStore::createIndex %s (keyPath: %s, unique: %i, multiEntry: %i)", name.utf8().data(), loggingString(keyPath).utf8().data(), parameters.unique, parameters.multiEntry);
     ASSERT(currentThread() == m_transaction.database().originThreadID());
 
     if (!m_transaction.isVersionChange())
index 6c44e76..27290f4 100644 (file)
@@ -56,7 +56,7 @@ String IDBIndexInfo::loggingString(int indent) const
     for (int i = 0; i < indent; ++i)
         indentString.append(" ");
 
-    return makeString(indentString, "Index: ", m_name, String::format(" (%" PRIu64 ") \n", m_identifier));
+    return makeString(indentString, "Index: ", m_name, String::format(" (%" PRIu64 ") keyPath: %s\n", m_identifier, WebCore::loggingString(m_keyPath).utf8().data()));
 }
 
 String IDBIndexInfo::condensedLoggingString() const
index 6c1f460..8fee1e2 100644 (file)
@@ -393,7 +393,7 @@ static Vector<IDBKeyData> createKeyPathArray(ExecState& exec, JSValue value, con
         Vector<IDBKeyData> keys;
         for (auto& entry : vector) {
             auto key = internalCreateIDBKeyFromScriptValueAndKeyPath(exec, value, entry);
-            if (!key)
+            if (!key || !key->isValid())
                 return { };
             keys.append(key.get());
         }