Javascript using WebSQL can create their own WebKit info table.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Apr 2015 22:15:29 +0000 (22:15 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Apr 2015 22:15:29 +0000 (22:15 +0000)
<rdar://problem/20688792> and https://bugs.webkit.org/show_bug.cgi?id=144466

Reviewed by Alex Christensen.

Source/WebCore:

Test: storage/websql/alter-to-info-table.html

* Modules/webdatabase/DatabaseBackendBase.cpp:
(WebCore::DatabaseBackendBase::databaseInfoTableName): Return the info table name.
(WebCore::fullyQualifiedInfoTableName): Append "main." to the info table name.
(WebCore::DatabaseBackendBase::DatabaseBackendBase): Use the fully qualified name.
(WebCore::DatabaseBackendBase::performOpenAndVerify): Ditto.
(WebCore::DatabaseBackendBase::getVersionFromDatabase): Ditto.
(WebCore::DatabaseBackendBase::setVersionInDatabase): Ditto.

LayoutTests:

* storage/websql/alter-to-info-table-expected.txt: Added.
* storage/websql/alter-to-info-table.html: Added.
* storage/websql/alter-to-info-table.js: Added.

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

LayoutTests/ChangeLog
LayoutTests/storage/websql/alter-to-info-table-expected.txt [new file with mode: 0644]
LayoutTests/storage/websql/alter-to-info-table.html [new file with mode: 0644]
LayoutTests/storage/websql/alter-to-info-table.js [new file with mode: 0644]
LayoutTests/storage/websql/test-authorizer-expected.txt
LayoutTests/storage/websql/test-authorizer.js
Source/WebCore/ChangeLog
Source/WebCore/Modules/webdatabase/DatabaseBackendBase.cpp

index 310ec00..447e03a 100644 (file)
@@ -1,3 +1,14 @@
+2015-04-30  Brady Eidson  <beidson@apple.com>
+
+        Javascript using WebSQL can create their own WebKit info table.
+        <rdar://problem/20688792> and https://bugs.webkit.org/show_bug.cgi?id=144466
+
+        Reviewed by Alex Christensen.
+
+        * storage/websql/alter-to-info-table-expected.txt: Added.
+        * storage/websql/alter-to-info-table.html: Added.
+        * storage/websql/alter-to-info-table.js: Added.
+
 2015-04-30  Martin Robinson  <mrobinson@igalia.com>
 
         Unskip isolated words tests on WebKitGTK+
diff --git a/LayoutTests/storage/websql/alter-to-info-table-expected.txt b/LayoutTests/storage/websql/alter-to-info-table-expected.txt
new file mode 100644 (file)
index 0000000..244a997
--- /dev/null
@@ -0,0 +1,10 @@
+This tests that a tricky way to setup your own info table fails.
+CREATE TABLE statement succeeded.
+CREATE TEMP TABLE statement succeeded.
+INSERT IN TEMP TABLE statement succeeded.
+CREATE TRIGGER statement succeeded.
+ALTER TO INFO TABLE statement succeeded.
+Step 1 transaction succeeded.
+Successfully changed DB version
+Step 2 transaction succeeded.
+
diff --git a/LayoutTests/storage/websql/alter-to-info-table.html b/LayoutTests/storage/websql/alter-to-info-table.html
new file mode 100644 (file)
index 0000000..b88196b
--- /dev/null
@@ -0,0 +1,12 @@
+<html>
+<head>
+<script src="resources/database-common.js"></script>
+<script src="alter-to-info-table.js"></script>
+</head>
+<body onload="setupAndRunTest();">
+This tests that a tricky way to setup your own info table fails.<br>
+<pre id="console">
+FAILURE: test didn't run.
+</pre>
+</body>
+</html>
diff --git a/LayoutTests/storage/websql/alter-to-info-table.js b/LayoutTests/storage/websql/alter-to-info-table.js
new file mode 100644 (file)
index 0000000..afacb59
--- /dev/null
@@ -0,0 +1,112 @@
+function terminateTest()
+{
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+
+function logAndTerminateTest(message, error)
+{
+    log(message + ": " + error.message);
+    terminateTest();
+}
+
+function cleanup(db)
+{
+    db.transaction(function(tx) {
+        tx.executeSql("DROP TABLE IF EXISTS Results;");
+        tx.executeSql("DROP TABLE IF EXISTS TempTable;");
+        tx.executeSql("DROP TRIGGER IF EXISTS TempTrigger;");
+    },
+    function(error) { 
+        logAndTerminateTest("Cleanup failed", error); 
+    });
+}
+
+function statementSuccessCallback(statementType)
+{
+    log(statementType + " statement succeeded.");
+}
+
+function statementErrorCallback(statementType, error)
+{
+    log(statementType + " statement failed: " + error.message);
+    return false;
+}
+
+function executeStatement(tx, statement, operation)
+{
+    tx.executeSql(statement, [],
+    function(result) { 
+        statementSuccessCallback(operation);
+    },
+    function(tx, error) { 
+        return statementErrorCallback(operation, error); 
+    });
+}
+
+function testCallbacks(tx)
+{
+       executeStatement(tx, "CREATE TABLE Results (Key TEXT, Value TEXT);", "CREATE TABLE");
+
+       // Create a temporary table with the same schema as __WebKitDatabaseInfoTable__ and populate it with a valid version.
+       executeStatement(tx, "CREATE TEMPORARY TABLE TempTable (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE, value TEXT NOT NULL ON CONFLICT FAIL);", "CREATE TEMP TABLE");
+       executeStatement(tx, "INSERT INTO TempTable VALUES ('WebKitDatabaseVersionKey','1.0');", "INSERT IN TEMP TABLE");
+
+    // Set up a trigger to capture changes to the table we just created.
+       executeStatement(tx, "CREATE TRIGGER TempTrigger BEFORE INSERT ON TempTable BEGIN INSERT INTO Results VALUES (NEW.key, NEW.value); END;", "CREATE TRIGGER");
+       
+       // Try to spoof that table as the info table.
+       executeStatement(tx, "ALTER TABLE TempTable RENAME TO __WebKitDatabaseInfoTable__;", "ALTER TO INFO TABLE");
+}
+
+function testStep1(db)
+{
+    db.transaction(function(tx) {
+        testCallbacks(tx);
+    },
+    function(error) { 
+        logAndTerminateTest("Step 1 transaction failed", error); 
+    },
+    function() {
+        log("Step 1 transaction succeeded.");
+        testStep2(db); 
+    });        
+}
+
+function testStep2(db)
+{
+    // At this point there's a temporary table named the same as the internal info table.
+    // WebKit should not use it.
+    db.changeVersion('1.0', '2.0', null, function(error) {
+        log("Failed to change DB version - " + error.message);
+    }, 
+    function() {
+        log("Successfully changed DB version");
+    });
+    
+    // If our trigger fired it will have captured the changed to the info table and put them in the results table.
+    db.transaction(function(tx) {
+        tx.executeSql("SELECT * FROM Results;", [], function(tx, results) {
+            if (results.rows.length == 0)
+                return;
+            log("The Results table actually has stuff in it, and it shouldn't");
+            var result = results.rows.item(0);
+            for (n in result)
+                log(n + " " + result[n]);
+        }); 
+    },
+    function(error) { 
+        logAndTerminateTest("Step 2 transaction failed", error); 
+    },
+    function() { 
+        log("Step 2 transaction succeeded.");
+        terminateTest(); 
+    });        
+}
+
+function runTest()
+{
+    var db = openDatabaseWithSuffix("AlterInfoTableTest", "1.0", "Tests altering the info table", 32768);
+    cleanup(db);
+    testStep1(db);
+}
index bbfafd4..8e07090 100644 (file)
@@ -15,6 +15,9 @@ SQLITE_UPDATE statement succeeded.
 SQLITE_PRAGMA statement failed: could not prepare statement (23 not authorized)
 SQLITE_ALTER_TABLE statement succeeded.
 SQLITE_ALTER_TABLE statement succeeded.
+SQLITE_ALTER_INFO_TABLE statement failed: could not prepare statement (23 not authorized)
+SQLITE_ALTER_INFO_TABLE statement failed: could not prepare statement (23 not authorized)
+SQLITE_ALTER_INFO_TABLE statement failed: could not prepare statement (1 there is already another table or index with this name: __WebKitDatabaseInfoTable__)
 SQLITE_TRANSACTION statement failed: could not prepare statement (23 not authorized)
 SQLITE_ATTACH statement failed: could not prepare statement (23 not authorized)
 SQLITE_DETACH statement failed: could not prepare statement (23 not authorized)
@@ -52,6 +55,9 @@ SQLITE_UPDATE statement failed: could not prepare statement (23 not authorized)
 SQLITE_PRAGMA statement failed: could not prepare statement (23 not authorized)
 SQLITE_ALTER_TABLE statement failed: could not prepare statement (23 not authorized)
 SQLITE_ALTER_TABLE statement failed: could not prepare statement (1 no such table: TestTable)
+SQLITE_ALTER_INFO_TABLE statement failed: could not prepare statement (23 not authorized)
+SQLITE_ALTER_INFO_TABLE statement failed: could not prepare statement (23 not authorized)
+SQLITE_ALTER_INFO_TABLE statement failed: could not prepare statement (1 there is already another table or index with this name: __WebKitDatabaseInfoTable__)
 SQLITE_TRANSACTION statement failed: could not prepare statement (23 not authorized)
 SQLITE_ATTACH statement failed: could not prepare statement (23 not authorized)
 SQLITE_DETACH statement failed: could not prepare statement (23 not authorized)
index 17e7d11..9c3d720 100644 (file)
@@ -73,6 +73,11 @@ function otherStatementsCallback(tx)
     // Rename the table back to its original name
     executeStatement(tx, "ALTER TABLE TestTable RENAME To Test;", "SQLITE_ALTER_TABLE");
 
+    // These should always fail, as nobody gets to mess with the info table.
+    executeStatement(tx, "ALTER TABLE __WebKitDatabaseInfoTable__ RENAME TO TestTable;", "SQLITE_ALTER_INFO_TABLE");
+    executeStatement(tx, "ALTER TABLE main.__WebKitDatabaseInfoTable__ RENAME TO TestTable;", "SQLITE_ALTER_INFO_TABLE");
+    executeStatement(tx, "ALTER TABLE Test RENAME TO __WebKitDatabaseInfoTable__;", "SQLITE_ALTER_INFO_TABLE");
+
     executeStatement(tx, "BEGIN TRANSACTION;", "SQLITE_TRANSACTION");
     executeStatement(tx, "ATTACH main AS TestMain;", "SQLITE_ATTACH");
     executeStatement(tx, "DETACH TestMain;", "SQLITE_DETACH");
index e515954..a1854aa 100644 (file)
@@ -1,3 +1,20 @@
+2015-04-30  Brady Eidson  <beidson@apple.com>
+
+        Javascript using WebSQL can create their own WebKit info table.
+        <rdar://problem/20688792> and https://bugs.webkit.org/show_bug.cgi?id=144466
+
+        Reviewed by Alex Christensen.
+
+        Test: storage/websql/alter-to-info-table.html
+
+        * Modules/webdatabase/DatabaseBackendBase.cpp:
+        (WebCore::DatabaseBackendBase::databaseInfoTableName): Return the info table name.
+        (WebCore::fullyQualifiedInfoTableName): Append "main." to the info table name.
+        (WebCore::DatabaseBackendBase::DatabaseBackendBase): Use the fully qualified name.
+        (WebCore::DatabaseBackendBase::performOpenAndVerify): Ditto.
+        (WebCore::DatabaseBackendBase::getVersionFromDatabase): Ditto.
+        (WebCore::DatabaseBackendBase::setVersionInDatabase): Ditto.
+
 2015-04-30  Beth Dakin  <bdakin@apple.com>
 
         Should choose UIScrollView indicatorStyle based on the document background color
index 9d63f09..be18851 100644 (file)
 namespace WebCore {
 
 static const char versionKey[] = "WebKitDatabaseVersionKey";
-static const char infoTableName[] = "__WebKitDatabaseInfoTable__";
+static const char unqualifiedInfoTableName[] = "__WebKitDatabaseInfoTable__";
+
+const char* DatabaseBackendBase::databaseInfoTableName()
+{
+    return unqualifiedInfoTableName;
+}
+
+static const char* fullyQualifiedInfoTableName()
+{
+    static const char qualifier[] = "main.";
+    static char qualifiedName[sizeof(qualifier) + sizeof(unqualifiedInfoTableName) - 1];
+
+    static std::once_flag onceFlag;
+    std::call_once(onceFlag, []{
+        char* newDestination = stpcpy(qualifiedName, qualifier);
+        strcpy(newDestination, unqualifiedInfoTableName);
+    });
+
+    return qualifiedName;
+}
 
 static String formatErrorMessage(const char* message, int sqliteErrorCode, const char* sqliteErrorMessage)
 {
@@ -190,12 +209,6 @@ static DatabaseGuid guidForOriginAndName(const String& origin, const String& nam
     return guid;
 }
 
-// static
-const char* DatabaseBackendBase::databaseInfoTableName()
-{
-    return infoTableName;
-}
-
 #if !LOG_DISABLED || !ERROR_DISABLED
 String DatabaseBackendBase::databaseDebugName() const
 {
@@ -214,7 +227,7 @@ DatabaseBackendBase::DatabaseBackendBase(PassRefPtr<DatabaseContext> databaseCon
 {
     m_contextThreadSecurityOrigin = m_databaseContext->securityOrigin()->isolatedCopy();
 
-    m_databaseAuthorizer = DatabaseAuthorizer::create(infoTableName);
+    m_databaseAuthorizer = DatabaseAuthorizer::create(unqualifiedInfoTableName);
 
     if (m_name.isNull())
         m_name = emptyString();
@@ -344,7 +357,7 @@ bool DatabaseBackendBase::performOpenAndVerify(bool shouldSetVersionInNewDatabas
                 return false;
             }
 
-            String tableName(infoTableName);
+            String tableName(unqualifiedInfoTableName);
             if (!m_sqliteDatabase.tableExists(tableName)) {
                 m_new = true;
 
@@ -443,7 +456,7 @@ DatabaseDetails DatabaseBackendBase::details() const
 
 bool DatabaseBackendBase::getVersionFromDatabase(String& version, bool shouldCacheVersion)
 {
-    String query(String("SELECT value FROM ") + infoTableName +  " WHERE key = '" + versionKey + "';");
+    String query(String("SELECT value FROM ") + fullyQualifiedInfoTableName() +  " WHERE key = '" + versionKey + "';");
 
     m_databaseAuthorizer->disable();
 
@@ -463,7 +476,7 @@ bool DatabaseBackendBase::setVersionInDatabase(const String& version, bool shoul
 {
     // The INSERT will replace an existing entry for the database with the new version number, due to the UNIQUE ON CONFLICT REPLACE
     // clause in the CREATE statement (see Database::performOpenAndVerify()).
-    String query(String("INSERT INTO ") + infoTableName +  " (key, value) VALUES ('" + versionKey + "', ?);");
+    String query(String("INSERT INTO ") + fullyQualifiedInfoTableName() +  " (key, value) VALUES ('" + versionKey + "', ?);");
 
     m_databaseAuthorizer->disable();