From 9aeb5052c62af64ae08a87b8b45e407aba585d81 Mon Sep 17 00:00:00 2001 From: cgivre Date: Wed, 9 Sep 2026 09:58:31 -0400 Subject: [PATCH] DRILL-8554: Update Jackcess and Fix Linked Table Issue --- contrib/format-access/README.md | 22 ++++++++++ contrib/format-access/pom.xml | 2 +- .../store/msaccess/MSAccessBatchReader.java | 17 +++++++- .../store/msaccess/MSAccessFormatConfig.java | 16 ++++++-- .../store/msaccess/TestMSAccessReader.java | 41 +++++++++++++++++++ 5 files changed, 93 insertions(+), 5 deletions(-) diff --git a/contrib/format-access/README.md b/contrib/format-access/README.md index 0c3035117aa..ab210a0e727 100644 --- a/contrib/format-access/README.md +++ b/contrib/format-access/README.md @@ -11,6 +11,28 @@ Simply add the following to any Drill file system configuration. Typically, MS } ``` +### Linked Tables +An Access file can define tables that live in another database file, referenced by an absolute local path or a UNC +share embedded in the file itself. Drill refuses to follow those references by default: reading an untrusted file +would otherwise make the Drillbit open a path of the file author's choosing, as the Drillbit service account. + +If you trust the files being queried and need linked tables to work, set `allowLinkedDatabases` to `true`: + +```json +"msaccess": { + "type": "msaccess", + "extensions": ["mdb", "accdb"], + "allowLinkedDatabases": true +} +``` + +It can also be enabled per query: + +```sql +SELECT * +FROM table(dfs.`file_name.accdb` (type=> 'msaccess', tableName => 'Linked', allowLinkedDatabases => true)) +``` + ## Schemas Drill will discover the schema automatically from the Access file. The plugin does support schema provisioning for consistency, but is not recommended. diff --git a/contrib/format-access/pom.xml b/contrib/format-access/pom.xml index 288bc9d92d9..82621466717 100644 --- a/contrib/format-access/pom.xml +++ b/contrib/format-access/pom.xml @@ -39,7 +39,7 @@ com.healthmarketscience.jackcess jackcess - 4.0.8 + 4.0.11 commons-logging diff --git a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java index 735c2cc71fc..9989e95b53d 100644 --- a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java +++ b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java @@ -22,6 +22,7 @@ import com.healthmarketscience.jackcess.DataType; import com.healthmarketscience.jackcess.Database; import com.healthmarketscience.jackcess.DatabaseBuilder; +import com.healthmarketscience.jackcess.util.LinkResolver; import com.healthmarketscience.jackcess.Row; import com.healthmarketscience.jackcess.Table; import org.apache.commons.lang3.StringUtils; @@ -58,6 +59,15 @@ public class MSAccessBatchReader implements ManagedReader { private static final Logger logger = LoggerFactory.getLogger(MSAccessBatchReader.class); + /** + * An Access file can name another database (a local path or a UNC share) as the source of a + * linked table. Jackcess's default resolver would open it as the Drillbit service account, so + * refuse instead unless the plugin config explicitly opts in. + */ + private static final LinkResolver REJECT_LINKED_DATABASES = (linkerDb, linkeeFileName) -> { + throw new IOException("Refusing to open linked database referenced by this MS Access file: " + linkeeFileName); + }; + private final FileDescrip file; private final CustomErrorContext errorContext; private final RowSetLoader rowWriter; @@ -247,7 +257,12 @@ public boolean next() { private void openFile() { try { fsStream = file.fileSystem().openPossiblyCompressedStream(file.split().getPath()); - db = DatabaseBuilder.open(convertInputStreamToFile(fsStream)); + db = new DatabaseBuilder(convertInputStreamToFile(fsStream)) + .setReadOnly(true) + .open(); + if (!config.getAllowLinkedDatabases()) { + db.setLinkResolver(REJECT_LINKED_DATABASES); + } tableList = db.getTableNames(); } catch (IOException e) { deleteTempFile(); diff --git a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java index 3a498f7b222..74bf434ba65 100644 --- a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java +++ b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java @@ -36,13 +36,16 @@ public class MSAccessFormatConfig implements FormatPluginConfig { private final List extensions; private final String tableName; + private final boolean allowLinkedDatabases; // Omitted properties take reasonable defaults @JsonCreator public MSAccessFormatConfig(@JsonProperty("extensions") List extensions, - @JsonProperty("tableName") String tableName) { + @JsonProperty("tableName") String tableName, + @JsonProperty("allowLinkedDatabases") Boolean allowLinkedDatabases) { this.extensions = extensions == null ? Arrays.asList("accdb", "mdb") : ImmutableList.copyOf(extensions); this.tableName = tableName; + this.allowLinkedDatabases = allowLinkedDatabases != null && allowLinkedDatabases; } @JsonInclude(Include.NON_DEFAULT) @@ -55,6 +58,11 @@ public String getTableName() { return tableName; } + @JsonInclude(Include.NON_DEFAULT) + public boolean getAllowLinkedDatabases() { + return allowLinkedDatabases; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -65,12 +73,13 @@ public boolean equals(Object o) { } MSAccessFormatConfig that = (MSAccessFormatConfig) o; return Objects.equals(extensions, that.extensions) && - Objects.equals(tableName, that.tableName); + Objects.equals(tableName, that.tableName) && + allowLinkedDatabases == that.allowLinkedDatabases; } @Override public int hashCode() { - return Objects.hash(extensions, tableName); + return Objects.hash(extensions, tableName, allowLinkedDatabases); } @Override @@ -78,6 +87,7 @@ public String toString() { return new PlanStringBuilder(this) .field("extensions", extensions) .field("tableName", tableName) + .field("allowLinkedDatabases", allowLinkedDatabases) .toString(); } } diff --git a/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java b/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java index e05e57f814f..085206597f6 100644 --- a/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java +++ b/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java @@ -18,6 +18,8 @@ package org.apache.drill.exec.store.msaccess; +import com.healthmarketscience.jackcess.Database; +import com.healthmarketscience.jackcess.DatabaseBuilder; import org.apache.drill.categories.RowSetTest; import org.apache.drill.common.types.TypeProtos.MinorType; import org.apache.drill.exec.physical.rowSet.RowSet; @@ -33,8 +35,12 @@ import org.junit.experimental.categories.Category; +import java.io.File; + import static org.apache.drill.test.rowSet.RowSetUtilities.strArray; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; @Category(RowSetTest.class) public class TestMSAccessReader extends ClusterTest { @@ -44,6 +50,41 @@ public static void setup() throws Exception { ClusterTest.startCluster(ClusterFixture.builder(dirTestWatcher)); } + @Test + public void testLinkedTableResolvedWhenAllowed() throws Exception { + String sql = "SELECT * FROM table(dfs.`" + writeLinkedTableFile("linked_allowed.mdb").getName() + + "` (type=> 'msaccess', tableName => 'Linked', allowLinkedDatabases => true))"; + try { + client.queryBuilder().sql(sql).run(); + fail("Expected the (nonexistent) linked database to be opened"); + } catch (Exception e) { + // Jackcess got as far as trying to open the linked path, which is what the option enables. + assertTrue(e.getMessage(), e.getMessage().contains("given file does not exist")); + } + } + + private static File writeLinkedTableFile(String name) throws Exception { + File mdb = new File(dirTestWatcher.getRootDir(), name); + try (Database db = DatabaseBuilder.create(Database.FileFormat.V2003, mdb)) { + db.createLinkedTable("Linked", "//evil.example.com/share/secret.mdb", "Table1"); + } + return mdb; + } + + @Test + public void testLinkedTableIsNotResolved() throws Exception { + // A malicious file can point a linked table at any local path or UNC share; reading it must + // not make the Drillbit open that path. + File mdb = writeLinkedTableFile("linked.mdb"); + String sql = "SELECT * FROM table(dfs.`" + mdb.getName() + "` (type=> 'msaccess', tableName => 'Linked'))"; + try { + client.queryBuilder().sql(sql).run(); + fail("Expected the linked database to be refused"); + } catch (Exception e) { + assertTrue(e.getMessage(), e.getMessage().contains("Refusing to open linked database")); + } + } + @Test public void testStarQuery() throws Exception { String sql = "SELECT * FROM table(cp.`data/V2019/extDateTestV2019.accdb` (type=> 'msaccess', tableName => 'Table1')) LIMIT 5";