(core) forbid use of sqlite ATTACH except during VACUUM

Summary:
This calls sqlite3_limit(SQLITE_LIMIT_ATTACHED, 0) so that
if ever an `ATTACH` were snuck into an SQL query, it would be denied.
The limit needs to be waived when calling VACUUM since the implementation
of VACUUM uses ATTACH.

Test Plan: added test; existing tests should pass

Reviewers: alexmojaki

Reviewed By: alexmojaki

Subscribers: alexmojaki

Differential Revision: https://phab.getgrist.com/D3316

app/server/lib/DocWorker.ts

@@ -193,6 +193,6 @@ async function removeData(filename: string) {
   }
   const history = new ActionHistoryImpl(db);
   await history.deleteActions(1);
-  await db.run('VACUUM');
+  await db.vacuum();
   await db.close();
 }

app/server/lib/SQLiteDB.ts

@@ -202,7 +202,7 @@ export class SQLiteDB {
 
     let _db: sqlite3.Database;
     await fromCallback(cb => { _db = new sqlite3.Database(dbPath, sqliteMode, cb); });
-
+    limitAttach(_db!, 0);  // Outside of VACUUM, we don't allow ATTACH.
     if (SQLiteDB._addOpens(dbPath, 1) > 1) {
       log.warn("SQLiteDB[%s] avoid opening same DB more than once", dbPath);
     }
@@ -319,12 +319,21 @@ export class SQLiteDB {
       this._needVacuum = true;
       return false;
     }
-    await this.exec("VACUUM");
+    await this.vacuum();
     log.info("SQLiteDB[%s]: DB VACUUMed", this._dbPath);
     this._needVacuum = false;
     return true;
   }
 
+  public async vacuum(): Promise<void> {
+    limitAttach(this._db, 1);  // VACUUM implementation uses ATTACH.
+    try {
+      await this.exec("VACUUM");
+    } finally {
+      limitAttach(this._db, 0);  // Outside of VACUUM, we don't allow ATTACH.
+    }
+  }
+
   /**
    * Run each of the statements in turn. Each statement is either a string, or an array of arguments
    * to db.run, e.g. [sqlString, [params...]].
@@ -480,7 +489,7 @@ export class SQLiteDB {
         });
         success = true;
         // After a migration, reduce the sqlite file size. This must be run outside a transaction.
-        await this.run("VACUUM");
+        await this.vacuum();
 
         log.info("SQLiteDB[%s]: DB backed up to %s, migrated to %s",
           this._dbPath, backupPath, targetVer);
@@ -547,3 +556,12 @@ export function quoteIdent(ident: string): string {
   assert(/^[\w.]+$/.test(ident), `SQL identifier is not valid: ${ident}`);
   return `"${ident}"`;
 }
+
+/**
+ * Limit the number of ATTACHed databases permitted.
+ */
+export function limitAttach(db: sqlite3.Database, maxAttach: number) {
+  // Pardon the casts, types are out of date.
+  const SQLITE_LIMIT_ATTACHED = (sqlite3 as any).LIMIT_ATTACHED;
+  (db as any).configure('limit', SQLITE_LIMIT_ATTACHED, maxAttach);
+}

package.json

@@ -80,7 +80,7 @@
     "@gristlabs/express-session": "1.17.0",
     "@gristlabs/moment-guess": "1.2.4-grist.1",
     "@gristlabs/pidusage": "2.0.17",
-    "@gristlabs/sqlite3": "4.1.1-grist.4",
+    "@gristlabs/sqlite3": "4.1.1-grist.6",
     "@popperjs/core": "2.3.3",
     "accept-language-parser": "1.5.0",
     "async-mutex": "0.2.4",

yarn.lock

@@ -77,10 +77,10 @@
   dependencies:
     safe-buffer "^5.1.2"
 
-"@gristlabs/sqlite3@4.1.1-grist.4", "@gristlabs/sqlite3@^4.1.1-grist.4":
-  version "4.1.1-grist.4"
-  resolved "https://registry.yarnpkg.com/@gristlabs/sqlite3/-/sqlite3-4.1.1-grist.4.tgz#b909983a33ac66f4a086a18318389ef34c779720"
-  integrity sha512-/PzqeZJm9bNaqP1hsj3bt0E5q1VFdZHDnqzLYxSJzDl0rxVdGkkOmnlW4cAwjyRCMyvpeXBVAY6U4BS2xehHSg==
+"@gristlabs/sqlite3@4.1.1-grist.6", "@gristlabs/sqlite3@^4.1.1-grist.4":
+  version "4.1.1-grist.6"
+  resolved "https://registry.yarnpkg.com/@gristlabs/sqlite3/-/sqlite3-4.1.1-grist.6.tgz#2a63021fec708e5165d0eeba19777a8c0ea3170d"
+  integrity sha512-XtkwvnicrgcbncraS+IkZtDS4mUplJG+8nzJp5cX0Q7NFhrigjzmXX59aJMx7dGrCBuR6GCmlszRCiONm5IMrQ==
   dependencies:
     node-addon-api "2.0.0"
     node-pre-gyp "^0.11.0"