1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181
|
From: Markus Koschany <apo@debian.org>
Date: Sat, 27 Sep 2025 15:59:27 +0200
Subject: CVE-2024-47881
Origin: https://github.com/OpenRefine/OpenRefine/commit/8a5cced755f9d4544cfc9fd1b9dc9274807b5020
Bug-Debian: https://bugs.debian.org/1086041
---
.../database/sqlite/SQLiteConnectionManager.java | 16 ++++++++-
extensions/database/tests/conf/tests.xml | 2 +-
.../extension/database/DBExtensionTests.java | 2 +-
.../sqlite/SQLiteConnectionManagerTest.java | 10 ------
.../database/sqlite/SQLiteDatabaseServiceTest.java | 42 +++++++++++++++-------
5 files changed, 47 insertions(+), 25 deletions(-)
diff --git a/extensions/database/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManager.java b/extensions/database/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManager.java
index 7d42e00..0def4d2 100644
--- a/extensions/database/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManager.java
+++ b/extensions/database/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManager.java
@@ -35,6 +35,7 @@ import com.google.refine.extension.database.SQLType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.File;
import java.net.URI;
import java.net.URISyntaxException;
import java.sql.Connection;
@@ -68,8 +69,21 @@ public class SQLiteConnectionManager {
}
public static String getDatabaseUrl(DatabaseConfiguration dbConfig) {
+ String dbPath = dbConfig.getDatabaseName();
+ if (dbPath.contains("?")) {
+ throw new IllegalArgumentException("Paths to SQLite databases are not allowed to contain '?'");
+ }
+ if (dbPath.startsWith("//") || dbPath.startsWith("\\\\") || dbPath.startsWith("\\/") || dbPath.startsWith("/\\")) {
+ throw new IllegalArgumentException("File path starts with illegal prefix; only local files are accepted.");
+ }
+ if (!new File(dbPath).isFile()) {
+ throw new IllegalArgumentException("File could not be read: " + dbPath);
+ }
try {
- URI uri = new URI("jdbc:" + dbConfig.getDatabaseType().toLowerCase(), dbConfig.getDatabaseName(), null);
+ URI uri = new URI(
+ "jdbc:" + dbConfig.getDatabaseType().toLowerCase(),
+ dbPath + "?open_mode=1&limit_attached=0", // open_mode=1 means read-only
+ null);
return uri.toASCIIString();
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
diff --git a/extensions/database/tests/conf/tests.xml b/extensions/database/tests/conf/tests.xml
index 7885174..a0910c9 100644
--- a/extensions/database/tests/conf/tests.xml
+++ b/extensions/database/tests/conf/tests.xml
@@ -29,7 +29,7 @@
<parameter name = "mariadbDbPassword" value=""/>
<parameter name = "mariadbTestTable" value="test_table"/>
- <parameter name = "sqliteDbName" value="extension_test_db.sqlite"/>
+ <parameter name = "sqliteDbName" value="tests/resources/test_db.sqlite"/>
<parameter name = "sqliteDbHost" value=""/>
<parameter name = "sqliteDbPort" value=""/>
<parameter name = "sqliteDbUser" value=""/>
diff --git a/extensions/database/tests/src/com/google/refine/extension/database/DBExtensionTests.java b/extensions/database/tests/src/com/google/refine/extension/database/DBExtensionTests.java
index bd1d0bc..a98f55a 100644
--- a/extensions/database/tests/src/com/google/refine/extension/database/DBExtensionTests.java
+++ b/extensions/database/tests/src/com/google/refine/extension/database/DBExtensionTests.java
@@ -61,7 +61,7 @@ public class DBExtensionTests {
protected final String DEFAULT_MARIADB_NAME = "testdb";
protected final String SQLITE_DB_NAME = "sqlite";
- protected final String DEFAULT_SQLITE_DB_NAME = "extension_test_db.sqlite";
+ protected final String DEFAULT_SQLITE_DB_NAME = "tests/resources/test_db.sqlite";
protected final String DEFAULT_TEST_TABLE = "test_data";
diff --git a/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManagerTest.java b/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManagerTest.java
index 2daf059..5afcdcb 100644
--- a/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManagerTest.java
+++ b/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteConnectionManagerTest.java
@@ -37,7 +37,6 @@ import org.mockito.MockitoAnnotations;
import org.testng.Assert;
import org.testng.annotations.*;
-import java.io.File;
import java.sql.Connection;
import java.sql.SQLException;
@@ -61,15 +60,6 @@ public class SQLiteConnectionManagerTest extends DBExtensionTests {
DatabaseService.DBType.registerDatabase(SQLiteDatabaseService.DB_NAME, SQLiteDatabaseService.getInstance());
}
- @AfterTest
- @Parameters({ "sqliteDbName" })
- public void afterTest(@Optional(DEFAULT_SQLITE_DB_NAME) String sqliteDbName) {
- File f = new File(sqliteDbName);
- if (f.exists()) {
- f.delete();
- }
- }
-
@Test
public void testTestConnection() throws DatabaseServiceException {
boolean isConnected = SQLiteConnectionManager.getInstance().testConnection(testDbConfig);
diff --git a/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteDatabaseServiceTest.java b/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteDatabaseServiceTest.java
index c29cc82..cd6f244 100644
--- a/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteDatabaseServiceTest.java
+++ b/extensions/database/tests/src/com/google/refine/extension/database/sqlite/SQLiteDatabaseServiceTest.java
@@ -37,7 +37,6 @@ import org.mockito.MockitoAnnotations;
import org.testng.Assert;
import org.testng.annotations.*;
-import java.io.File;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.List;
@@ -60,20 +59,10 @@ public class SQLiteDatabaseServiceTest extends DBExtensionTests {
testDbConfig.setDatabaseType(SQLiteDatabaseService.DB_NAME);
testTable = sqliteTestTable;
- DBExtensionTestUtils.initTestData(testDbConfig, sqliteTestTable);
DatabaseService.DBType.registerDatabase(SQLiteDatabaseService.DB_NAME, SQLiteDatabaseService.getInstance());
}
- @AfterTest
- @Parameters({ "sqliteDbName" })
- public void afterTest(@Optional(DEFAULT_SQLITE_DB_NAME) String sqliteDbName) {
- File f = new File(sqliteDbName);
- if (f.exists()) {
- f.delete();
- }
- }
-
@Test
public void testGetDatabaseUrl() {
SQLiteDatabaseService sqLiteDatabaseService = (SQLiteDatabaseService) DatabaseService
@@ -81,7 +70,7 @@ public class SQLiteDatabaseServiceTest extends DBExtensionTests {
String dbUrl = sqLiteDatabaseService.getDatabaseUrl(testDbConfig);
Assert.assertNotNull(dbUrl);
- Assert.assertEquals(dbUrl, "jdbc:sqlite:extension_test_db.sqlite");
+ Assert.assertEquals(dbUrl, "jdbc:sqlite:tests/resources/test_db.sqlite?open_mode=1&limit_attached=0");
}
@Test
@@ -94,6 +83,35 @@ public class SQLiteDatabaseServiceTest extends DBExtensionTests {
Assert.assertNotNull(conn);
}
+ /*
+ * We don't allow loading extensions because that executes arbitrary code
+ */
+ @Test(expectedExceptions = IllegalArgumentException.class)
+ public void testGetConnectionWithExtensions() throws DatabaseServiceException {
+ DatabaseConfiguration testDbConfigWithExtensions = new DatabaseConfiguration();
+ testDbConfigWithExtensions.setDatabaseName("test_db.sqlite?enable_load_extension=true");
+ testDbConfigWithExtensions.setDatabaseType(SQLiteDatabaseService.DB_NAME);
+
+ SQLiteDatabaseService sqLiteDatabaseService = (SQLiteDatabaseService) DatabaseService
+ .get(SQLiteDatabaseService.DB_NAME);
+ sqLiteDatabaseService.getConnection(testDbConfigWithExtensions);
+ }
+
+ /*
+ * We don't allow loading a remote SQLite file to make remote code execution harder
+ */
+ @Test(expectedExceptions = IllegalArgumentException.class)
+ public void testGetConnectionWithRemoteFile() throws DatabaseServiceException {
+ DatabaseConfiguration testDbConfigWithExtensions = new DatabaseConfiguration();
+ testDbConfigWithExtensions
+ .setDatabaseName("https://github.com/xerial/sqlite-jdbc/raw/master/src/test/resources/org/sqlite/sample.db");
+ testDbConfigWithExtensions.setDatabaseType(SQLiteDatabaseService.DB_NAME);
+
+ SQLiteDatabaseService sqLiteDatabaseService = (SQLiteDatabaseService) DatabaseService
+ .get(SQLiteDatabaseService.DB_NAME);
+ sqLiteDatabaseService.getConnection(testDbConfigWithExtensions);
+ }
+
@Test
public void testTestConnection() throws DatabaseServiceException {
SQLiteDatabaseService sqLiteDatabaseService = (SQLiteDatabaseService) DatabaseService
|