File: CVE-2024-47881.patch

package info (click to toggle)
openrefine 3.6.2-2%2Bdeb12u3
  • links: PTS, VCS
  • area: main
  • in suites: bookworm-proposed-updates
  • size: 44,192 kB
  • sloc: javascript: 95,878; java: 80,800; xml: 5,881; sh: 791; makefile: 65; sql: 60
file content (181 lines) | stat: -rw-r--r-- 9,163 bytes parent folder | download
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