diff --git a/changelog.d/unreleased/3220.security.md b/changelog.d/unreleased/3220.security.md new file mode 100644 index 0000000000..46f4564c0d --- /dev/null +++ b/changelog.d/unreleased/3220.security.md @@ -0,0 +1,20 @@ +--- +category: security +issues: + - 3220 +affected: + - src/CodeIndex/Cli/DbPathResolver.cs + - src/CodeIndex/Cli/DbCommandRunner.cs + - src/CodeIndex/Cli/DiffCommandRunner.cs + - src/CodeIndex/Database/DbContext.cs + - tests/CodeIndex.Tests/DbPathResolverTests.cs + - tests/CodeIndex.Tests/DbCommandRunnerTests.cs +--- + +## English + +- **SQLite file URI database opens now avoid connection-string injection (#3220)** — user-supplied `file:` URI database paths are built with `SqliteConnectionStringBuilder`, so `;Mode=...` payloads stay inside the data source value instead of becoming connection options. + +## 日本語 + +- **SQLite file URI の DB open で connection-string injection を防止しました (#3220)** — ユーザー指定の `file:` URI DB パスは `SqliteConnectionStringBuilder` で組み立て、`;Mode=...` payload が接続オプションではなく data source 値に留まるようにしました。 diff --git a/src/CodeIndex/Cli/DbCommandRunner.cs b/src/CodeIndex/Cli/DbCommandRunner.cs index 53ab56ec63..63f7dea9c1 100644 --- a/src/CodeIndex/Cli/DbCommandRunner.cs +++ b/src/CodeIndex/Cli/DbCommandRunner.cs @@ -422,13 +422,7 @@ private static DbIntegrityCheckReadResult RunIntegrityCheckPragma(string dbPath) if (IntegrityCheckRowsForTesting != null) return BoundIntegrityRows(IntegrityCheckRowsForTesting()); - var connectionString = dbPath.StartsWith("file:", StringComparison.OrdinalIgnoreCase) - ? $"Data Source={dbPath}" - : new SqliteConnectionStringBuilder - { - DataSource = dbPath, - Mode = SqliteOpenMode.ReadOnly, - }.ConnectionString; + var connectionString = DbPathResolver.BuildSqliteConnectionString(dbPath, SqliteOpenMode.ReadOnly); using var connection = new SqliteConnection(connectionString); connection.Open(); @@ -571,13 +565,9 @@ FROM reference_lines rl private static SqliteConnection OpenConnection(string dbPath, bool writable) { - var connectionString = dbPath.StartsWith("file:", StringComparison.OrdinalIgnoreCase) - ? $"Data Source={dbPath}" - : new SqliteConnectionStringBuilder - { - DataSource = dbPath, - Mode = writable ? SqliteOpenMode.ReadWrite : SqliteOpenMode.ReadOnly, - }.ConnectionString; + var connectionString = DbPathResolver.BuildSqliteConnectionString( + dbPath, + writable ? SqliteOpenMode.ReadWrite : SqliteOpenMode.ReadOnly); var connection = new SqliteConnection(connectionString); connection.Open(); return connection; diff --git a/src/CodeIndex/Cli/DbPathResolver.cs b/src/CodeIndex/Cli/DbPathResolver.cs index 1124e54045..b836182602 100644 --- a/src/CodeIndex/Cli/DbPathResolver.cs +++ b/src/CodeIndex/Cli/DbPathResolver.cs @@ -291,6 +291,17 @@ public static bool TryResolveWritableMutationDbPath(string dbPath, out string wr return true; } + internal static string BuildSqliteConnectionString(string dbPath, SqliteOpenMode? mode = null) + { + var builder = new SqliteConnectionStringBuilder + { + DataSource = dbPath, + }; + if (mode.HasValue) + builder.Mode = mode.Value; + return builder.ConnectionString; + } + private static string? TryReadIndexedProjectRoot(string dbPath) => TryReadMetaString(dbPath, CodeIndex.Database.DbContext.IndexedProjectRootMetaKey); @@ -412,15 +423,7 @@ private static bool SiblingRootMatchesIndexedContents(string dbPath, string full private static SqliteConnection OpenMetadataConnection(string dbPath) { - if (dbPath.StartsWith("file:", StringComparison.OrdinalIgnoreCase) && UriRequestsReadOnly(dbPath)) - return new SqliteConnection($"Data Source={dbPath}"); - - var builder = new SqliteConnectionStringBuilder - { - DataSource = dbPath, - Mode = SqliteOpenMode.ReadOnly, - }; - return new SqliteConnection(builder.ConnectionString); + return new SqliteConnection(BuildSqliteConnectionString(dbPath, SqliteOpenMode.ReadOnly)); } public static bool UriRequestsReadOnly(string uriText) diff --git a/src/CodeIndex/Cli/DiffCommandRunner.cs b/src/CodeIndex/Cli/DiffCommandRunner.cs index bb5f979637..2511388b99 100644 --- a/src/CodeIndex/Cli/DiffCommandRunner.cs +++ b/src/CodeIndex/Cli/DiffCommandRunner.cs @@ -626,13 +626,7 @@ private static SqliteConnection OpenReadOnlyConnection(string dbPath) if (!isUri && !File.Exists(LongPath.EnsureWindowsPrefix(dbPath))) throw new IOException($"database not found: {dbPath}"); - var connectionString = isUri - ? $"Data Source={dbPath}" - : new SqliteConnectionStringBuilder - { - DataSource = dbPath, - Mode = SqliteOpenMode.ReadOnly, - }.ConnectionString; + var connectionString = DbPathResolver.BuildSqliteConnectionString(dbPath, SqliteOpenMode.ReadOnly); var connection = new SqliteConnection(connectionString); connection.Open(); diff --git a/src/CodeIndex/Database/DbContext.cs b/src/CodeIndex/Database/DbContext.cs index 8b5eaab032..b60bd09ffd 100644 --- a/src/CodeIndex/Database/DbContext.cs +++ b/src/CodeIndex/Database/DbContext.cs @@ -252,7 +252,7 @@ public DbContext(string dbPath) { try { - _connection = new SqliteConnection($"Data Source={dbPath}"); + _connection = new SqliteConnection(DbPathResolver.BuildSqliteConnectionString(dbPath, SqliteOpenMode.ReadOnly)); _connection.Open(); Execute("PRAGMA busy_timeout=5000"); ApplyConnectionPerformancePragmas(); diff --git a/tests/CodeIndex.Tests/DbCommandRunnerTests.cs b/tests/CodeIndex.Tests/DbCommandRunnerTests.cs index f09f9fc848..02b4b65bbb 100644 --- a/tests/CodeIndex.Tests/DbCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/DbCommandRunnerTests.cs @@ -121,6 +121,27 @@ public void Run_MissingDb_ReturnsNotFoundWithHint() Assert.Contains("cdidx index ", stderr); } + [Fact] + public void Run_IntegrityCheck_FileUriSemicolonPayloadDoesNotCreateDatabase_Issue3220() + { + var missingDb = Path.Combine(Path.GetTempPath(), $"cdidx_db_uri_injection_{Guid.NewGuid():N}.db"); + var uri = new Uri(missingDb).AbsoluteUri + ";Mode=ReadWriteCreate"; + try + { + var (exitCode, _, stderr) = RunAndCaptureStreams(["--integrity-check", "--db", uri]); + + Assert.Equal(CommandExitCodes.DatabaseError, exitCode); + Assert.Contains("failed to run integrity check", stderr); + Assert.False(File.Exists(missingDb)); + } + finally + { + SqliteConnection.ClearAllPools(); + if (File.Exists(missingDb)) + File.Delete(missingDb); + } + } + [Fact] public void Run_MissingDb_JsonShapeIncludesHint() { diff --git a/tests/CodeIndex.Tests/DbPathResolverTests.cs b/tests/CodeIndex.Tests/DbPathResolverTests.cs index b81156b4ae..7655d4f15b 100644 --- a/tests/CodeIndex.Tests/DbPathResolverTests.cs +++ b/tests/CodeIndex.Tests/DbPathResolverTests.cs @@ -36,6 +36,19 @@ public void ResolveForIndex_PrefersExplicitPath() Assert.Equal(explicitPath, dbPath); } + [Fact] + public void BuildSqliteConnectionString_FileUriKeepsSemicolonPayloadInDataSource_Issue3220() + { + const string uri = "file:///tmp/codeindex.db?immutable=1;Mode=ReadWriteCreate;Cache=Shared"; + + var connectionString = DbPathResolver.BuildSqliteConnectionString(uri, SqliteOpenMode.ReadOnly); + var parsed = new SqliteConnectionStringBuilder(connectionString); + + Assert.Equal(uri, parsed.DataSource); + Assert.Equal(SqliteOpenMode.ReadOnly, parsed.Mode); + Assert.NotEqual(SqliteOpenMode.ReadWriteCreate, parsed.Mode); + } + [Fact] public void ResolveForIndex_PrefersExplicitDataDirWhenDbPathMissing() {