From e0c3812a251ffbde234aa4a16825a4d2f10d899b Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Thu, 7 Apr 2022 12:50:50 -0700
Subject: [PATCH] Disallow FDW connections in SQLite (#21525)
---
.../sqlite/src/metabase/driver/sqlite.clj | 4 ++-
.../test/metabase/driver/sqlite_test.clj | 29 ++++++++++++++++++-
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
index dc6f1b013cb..1286c093af2 100644
--- a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
+++ b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj
@@ -66,7 +66,9 @@
:as details}]
(merge {:subprotocol "sqlite"
:subname db}
- (dissoc details :db)))
+ (dissoc details :db)
+ ;; disallow "FDW" (connecting to other SQLite databases on the local filesystem) -- see https://github.com/metabase/metaboat/issues/152
+ {:limit_attached 0}))
;; We'll do regex pattern matching here for determining Field types because SQLite types can have optional lengths,
;; e.g. NVARCHAR(100) or NUMERIC(10,5) See also http://www.sqlite.org/datatype3.html
diff --git a/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj b/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
index cc5607a0727..67dd92c24d9 100644
--- a/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
+++ b/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
@@ -1,16 +1,19 @@
(ns metabase.driver.sqlite-test
- (:require [clojure.java.jdbc :as jdbc]
+ (:require [clojure.java.io :as io]
+ [clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
[metabase.driver :as driver]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql.query-processor-test-util :as sql.qp-test-util]
[metabase.models.database :refer [Database]]
[metabase.models.table :refer [Table]]
+ [metabase.query-processor :as qp]
[metabase.query-processor-test :as qp.test]
[metabase.sync :as sync]
[metabase.test :as mt]
[metabase.test.data :as data]
[metabase.test.util :as tu]
+ [metabase.util :as u]
[toucan.db :as db]
[toucan.hydrate :refer [hydrate]]))
@@ -232,3 +235,27 @@
:aggregation [:count]
:order-by [[:asc [:expression :CATEGORY]]]
:limit 1}))))))))
+
+(deftest disallow-fdw-to-other-databases-test
+ (testing "Don't allow connections to other SQLite databases with ATTACH DATABASE (https://github.com/metabase/metaboat/issues/152)"
+ (mt/test-driver :sqlite
+ ;; force creation of the sample dataset file
+ (mt/dataset sample-dataset
+ (mt/id))
+ (let [file (io/file "sample-dataset.sqlite")
+ path (.getAbsolutePath file)]
+ (is (.exists file))
+ (testing "Attach the sample dataset as an FDW called fdw_test"
+ (testing "Detach it if it already exists from a previous test run"
+ (u/ignore-exceptions
+ (qp/process-query (mt/native-query {:query "DETACH DATABASE fdw_test;"}))))
+ (testing "Attempting to attach it should fail"
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"SQL error or missing database \(too many attached databases - max 0\)"
+ (qp/process-query (mt/native-query {:query (format "ATTACH DATABASE 'file:%s' as fdw_test;" path)}))))))
+ (testing "Attempt to query the FDW -- shouldn't work"
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"SQL error or missing database \(no such table: fdw_test\.products\)"
+ (qp/process-query (mt/native-query {:query "SELECT count(*) FROM fdw_test.products;"})))))))))
--
GitLab