DRILL-8554: Update Jackcess and Fix Linked Table Issue

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.
 

contrib/format-access/pom.xml

@@ -39,7 +39,7 @@
         <dependency>
             <groupId>com.healthmarketscience.jackcess</groupId>
             <artifactId>jackcess</artifactId>
-            <version>4.0.8</version>
+            <version>4.0.11</version>
             <exclusions>
                 <exclusion>
                     <groupId>commons-logging</groupId>

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();

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<String> extensions;
   private final String tableName;
+  private final boolean allowLinkedDatabases;
 
   // Omitted properties take reasonable defaults
   @JsonCreator
   public MSAccessFormatConfig(@JsonProperty("extensions") List<String> 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,19 +73,21 @@ 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
   public String toString() {
     return new PlanStringBuilder(this)
         .field("extensions", extensions)
         .field("tableName", tableName)
+        .field("allowLinkedDatabases", allowLinkedDatabases)
         .toString();
   }
 }

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";