fixup! sqlite: reject connection access from authorizer callbacks

Cover the guard paths the existing tests never reached: the authorizer
guards on enableDefensive() and loadExtension(), the stepping guard on
statement[Symbol.dispose](), and the stepping guards on the tag store's
get(), all(), and iterate(). The tag store test looped over the four
outer driver methods but always reentered through run(), so three of its
four guards never fired.

loadExtension() checks that extension loading is enabled before the
authorizer guard, so its test opens the database with allowExtension.

Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Author: TrevorBurnham

test/parallel/test-sqlite-authz.js

@@ -344,12 +344,26 @@ suite('authorizer callback reentrancy', () => {
       function: () => db.function('noop', () => 1),
       aggregate: () => db.aggregate('agg', { start: 0, step: (acc) => acc }),
       enableLoadExtension: () => db.enableLoadExtension(false),
+      enableDefensive: () => db.enableDefensive(true),
       limits: () => { db.limits.length = 100; },
     };
 
     assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
   });
 
+  // loadExtension() checks that extension loading is enabled before reaching
+  // the authorizer guard, so it needs a database opened with allowExtension.
+  it('rejects loadExtension', () => {
+    const db = new DatabaseSync(':memory:', { allowExtension: true });
+    db.enableLoadExtension(true);
+    db.exec('CREATE TABLE t (x INTEGER)');
+    const cases = {
+      loadExtension: () => db.loadExtension('/nonexistent/extension'),
+    };
+
+    assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
+  });
+
   // close() and deserialize() tear down the connection, so the pre-existing
   // callback depth guard already rejects them with its own message.
   it('rejects methods the callback depth guard already covers', () => {
@@ -461,6 +475,36 @@ suite('authorizer callback reentrancy', () => {
     assert.strictEqual(outcome, steppingError);
   });
 
+  // Unlike an already-finalized statement, disposing the one being stepped
+  // would free the running virtual machine, so it throws.
+  it('rejects disposing the statement being stepped', () => {
+    const db = new DatabaseSync(':memory:');
+    db.exec('CREATE TABLE t (x INTEGER)');
+    db.exec('INSERT INTO t VALUES (1)');
+    const stmt = db.prepare('SELECT x FROM t');
+    stmt.get();
+    db.exec('ALTER TABLE t ADD COLUMN y INTEGER');
+
+    let outcome = 'authorizer callback did not run';
+    let ran = false;
+    db.setAuthorizer(() => {
+      if (!ran) {
+        ran = true;
+        try {
+          stmt[Symbol.dispose]();
+          outcome = 'did not throw';
+        } catch (err) {
+          outcome = `${err.code}: ${err.message}`;
+        }
+      }
+      return constants.SQLITE_OK;
+    });
+
+    stmt.get();
+
+    assert.strictEqual(outcome, steppingError);
+  });
+
   it('rejects iterator methods', () => {
     const db = new DatabaseSync(':memory:');
     db.exec('CREATE TABLE t (x INTEGER)');

test/parallel/test-sqlite-udf-close.js

@@ -114,47 +114,53 @@ for (const method of ['all', 'get', 'run', 'iterate']) {
   }
 
   // Tag store methods resolve to a cached statement, which may be the one
-  // currently being stepped.
-  test(`tag store reentry during statement.${method}()`, () => {
-    const db = new DatabaseSync(':memory:');
-    const sql = db.createTagStore(10);
-    db.exec(`
-      CREATE TABLE data (value INTEGER, padding TEXT);
-      INSERT INTO data VALUES (1, '${'x'.repeat(400)}'),
-                              (2, '${'y'.repeat(400)}');
-    `);
+  // currently being stepped. Each reentrant method has its own guard, so all
+  // four are exercised.
+  for (const reentrant of ['run', 'get', 'all', 'iterate']) {
+    test(`tag store ${reentrant} reentry during statement.${method}()`, () => {
+      const db = new DatabaseSync(':memory:');
+      const sql = db.createTagStore(10);
+      db.exec(`
+        CREATE TABLE data (value INTEGER, padding TEXT);
+        INSERT INTO data VALUES (1, '${'x'.repeat(400)}'),
+                                (2, '${'y'.repeat(400)}');
+      `);
 
-    let thrown;
-    db.function('reenter_tag', (value) => {
-      if (thrown === undefined) {
-        try {
-          // The identical tagged literal resolves to the same cached
-          // statement that is mid-execution.
-          // eslint-disable-next-line no-unused-expressions
-          sql.run`SELECT reenter_tag(value), padding FROM data`;
-          thrown = null;
-        } catch (err) {
-          thrown = err;
+      let thrown;
+      db.function('reenter_tag', (value) => {
+        if (thrown === undefined) {
+          try {
+            // The identical tagged literal resolves to the same cached
+            // statement that is mid-execution.
+            // All four reject at call time, iterate() included, so the
+            // result is never consumed.
+            // eslint-disable-next-line no-unused-expressions
+            sql[reentrant]`SELECT reenter_tag(value), padding FROM data`;
+            thrown = null;
+          } catch (err) {
+            thrown = err;
+          }
         }
-      }
-      return value;
-    });
+        return value;
+      });
 
-    if (method === 'iterate') {
-      for (const row of sql.iterate`SELECT reenter_tag(value), padding FROM data`) {
-        assert.ok(row);
+      if (method === 'iterate') {
+        for (const row of sql.iterate`SELECT reenter_tag(value), padding FROM data`) {
+          assert.ok(row);
+        }
+      } else {
+        // eslint-disable-next-line no-unused-expressions
+        sql[method]`SELECT reenter_tag(value), padding FROM data`;
       }
-    } else {
-      // eslint-disable-next-line no-unused-expressions
-      sql[method]`SELECT reenter_tag(value), padding FROM data`;
-    }
 
-    assert.ok(thrown, 'tag store reentry was not rejected');
-    assert.strictEqual(thrown.code, 'ERR_INVALID_STATE');
-    assert.strictEqual(thrown.message, 'statement is already being executed');
+      assert.ok(thrown, `tag store ${reentrant} reentry was not rejected`);
+      assert.strictEqual(thrown.code, 'ERR_INVALID_STATE');
+      assert.strictEqual(thrown.message,
+                         'statement is already being executed');
 
-    db.close();
-  });
+      db.close();
+    });
+  }
 
   // A UDF may prepare and finalize its own helper statements. Only the
   // statement being stepped is off limits.