From f4fe2cfaebce4a38ae4779705bf490325644dfe3 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:36:42 +0200 Subject: [PATCH] fix: reject SQL escape strings in read-only queries Reject PostgreSQL E/e escape-string literals at the shared read-only query boundary so protected-column inspection cannot be bypassed. Cover the reported exploit through DbAdmin and SqlKnowledgeReader, plus ordinary literals, comments and dollar quoting. Fixes #612 --- docs/ai-features.md | 6 +++++ .../DbAdmin/DbAdminReadOnlySqlGuard.cs | 3 +++ .../Security/SqlStatementInspector.cs | 14 ++++++++++++ .../Ai/SqlKnowledgeReaderTests.cs | 16 ++++++++++++++ .../Controllers/DbAdminControllerTests.cs | 16 ++++++++++++++ .../DbAdmin/DbAdminReadOnlySqlGuardTests.cs | 22 +++++++++++++++++++ 6 files changed, 77 insertions(+) diff --git a/docs/ai-features.md b/docs/ai-features.md index 659225a7..6d4ff371 100644 --- a/docs/ai-features.md +++ b/docs/ai-features.md @@ -117,6 +117,12 @@ begrenzt. Auch Bestandslisten zu Workflows, Läufen oder Maschinen verwenden die DB-Quelle. Ordnerrechte erweitern den Zugriff nicht auf SQL. Der Wissens-Assistent ändert keine Workflows und startet keine Reparaturen. +Der gemeinsame SQL-Guard für Wissensassistent und Admin-Konsole lehnt im Lesemodus +PostgreSQL-Escape-Strings (`E'…'`/`e'…'`) vor der Ausführung ausdrücklich ab. +Standard-SQL-Literale mit verdoppelten Apostrophen bleiben unterstützt. Die abweichende +[PostgreSQL-Escape-Syntax](https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-STRINGS-ESCAPE) +wird vom verwendeten Lexer nicht interpretiert. + ## Fehlerbehandlung | HTTP | Code | Ursache | diff --git a/src/NodePilot.Api/Services/DbAdmin/DbAdminReadOnlySqlGuard.cs b/src/NodePilot.Api/Services/DbAdmin/DbAdminReadOnlySqlGuard.cs index c91f7d00..fe3bba02 100644 --- a/src/NodePilot.Api/Services/DbAdmin/DbAdminReadOnlySqlGuard.cs +++ b/src/NodePilot.Api/Services/DbAdmin/DbAdminReadOnlySqlGuard.cs @@ -60,6 +60,9 @@ public static void Validate(string sql) if (sql.Length > DbAdminQueryExecutor.MaxSqlLength) throw new InvalidOperationException( $"SQL exceeds the {DbAdminQueryExecutor.MaxSqlLength}-character limit."); + if (SqlStatementInspector.ContainsEscapeStringLiteral(sql)) + throw new InvalidOperationException( + "Escape-string literals are not allowed in read mode. Use standard SQL string literals with doubled quotes."); if (DbAdminQueryExecutor.ContainsMultipleStatements(sql)) throw new InvalidOperationException("Read mode accepts exactly one SQL statement."); diff --git a/src/NodePilot.Core/Security/SqlStatementInspector.cs b/src/NodePilot.Core/Security/SqlStatementInspector.cs index 02f4c4d6..e2acc319 100644 --- a/src/NodePilot.Core/Security/SqlStatementInspector.cs +++ b/src/NodePilot.Core/Security/SqlStatementInspector.cs @@ -10,9 +10,12 @@ namespace NodePilot.Core.Security; public static class SqlStatementInspector { private const string UnicodeEscapedIdentifierMarker = ""; + private const string EscapeStringLiteralMarker = ""; private static readonly HashSet UnicodeEscapedIdentifierMarkers = new([UnicodeEscapedIdentifierMarker], StringComparer.Ordinal); + private static readonly HashSet EscapeStringLiteralMarkers = + new([EscapeStringLiteralMarker], StringComparer.Ordinal); private static readonly HashSet DynamicDataExporters = new(StringComparer.OrdinalIgnoreCase) { @@ -52,6 +55,10 @@ public static bool ReferencesAnyIdentifier(string sql, IReadOnlySet iden public static bool ContainsUnicodeEscapedIdentifier(string sql) => ReferencesAnyIdentifier(sql, UnicodeEscapedIdentifierMarkers); + /// PostgreSQL E-strings use backslash escaping, outside this lexer's literal grammar. + public static bool ContainsEscapeStringLiteral(string sql) + => ReferencesAnyIdentifier(sql, EscapeStringLiteralMarkers); + /// /// True when follows as consecutive /// unquoted identifier tokens. Punctuation and comments are ignored, matching normal SQL @@ -157,6 +164,13 @@ private static IEnumerable Tokenize(string sql, bool includeSynta continue; } + if (sql[i] is 'E' or 'e' && i + 1 < sql.Length && sql[i + 1] == '\'') + { + yield return new SqlIdentifier(EscapeStringLiteralMarker, Quoted: false); + // Unsupported syntax ends reliable inspection; the read guard rejects the marker. + yield break; + } + if (sql[i] == '\'') { SkipQuotedLiteral(sql, ref i, '\''); diff --git a/tests/NodePilot.Api.Tests/Ai/SqlKnowledgeReaderTests.cs b/tests/NodePilot.Api.Tests/Ai/SqlKnowledgeReaderTests.cs index 238b1f99..e64d00d7 100644 --- a/tests/NodePilot.Api.Tests/Ai/SqlKnowledgeReaderTests.cs +++ b/tests/NodePilot.Api.Tests/Ai/SqlKnowledgeReaderTests.cs @@ -22,6 +22,22 @@ namespace NodePilot.Api.Tests.Ai; /// public class SqlKnowledgeReaderTests { + [Fact] + public async Task ExecuteRead_EscapeStringBeforeAliasedSecret_IsRejectedBeforeDatabaseExecution() + { + using var db = TestDbFactory.Create(); + db.Users.Add(new User { Username = "admin", PasswordHash = "synthetic-secret-do-not-export" }); + await db.SaveChangesAsync(TestContext.Current.CancellationToken); + + var result = await NewReader(db).ExecuteReadAsync( + """SELECT E'\'' AS marker, "PasswordHash" AS harmless FROM "Users" -- '""", + TestContext.Current.CancellationToken); + + result.Error.Should().Contain("Escape-string literals are not allowed"); + result.Rows.Should().BeEmpty(); + result.Columns.Should().BeEmpty(); + } + private sealed class FakeScopeFactory : IServiceScopeFactory { private readonly NodePilotDbContext _db; diff --git a/tests/NodePilot.Api.Tests/Controllers/DbAdminControllerTests.cs b/tests/NodePilot.Api.Tests/Controllers/DbAdminControllerTests.cs index 0951150f..0348e84c 100644 --- a/tests/NodePilot.Api.Tests/Controllers/DbAdminControllerTests.cs +++ b/tests/NodePilot.Api.Tests/Controllers/DbAdminControllerTests.cs @@ -838,6 +838,22 @@ private sealed class AuditSaveTokenRecorder(string action) } } + [Fact] + public async Task ExecuteQuery_ReadMode_EscapeStringBeforeAliasedSecret_IsRejectedBeforeDatabaseExecution() + { + var (ctrl, db) = NewController(); + db.Users.Add(AdminUser()); + await db.SaveChangesAsync(TestContext.Current.CancellationToken); + + var result = await ctrl.ExecuteQuery(new DbAdminQueryRequest( + """SELECT E'\'' AS marker, "PasswordHash" AS harmless FROM "Users" -- '""", null), + TestContext.Current.CancellationToken); + + var error = result.Should().BeOfType().Subject.Value + .Should().BeOfType().Subject; + error.Message.Should().Contain("Escape-string literals are not allowed"); + } + // ── Secret-column containment (mirrors SqlKnowledgeReader; keeps the MCP run_readonly_sql // tool and the UI query pane from pulling secrets out of the DB) ────────────────────── diff --git a/tests/NodePilot.Api.Tests/Services/DbAdmin/DbAdminReadOnlySqlGuardTests.cs b/tests/NodePilot.Api.Tests/Services/DbAdmin/DbAdminReadOnlySqlGuardTests.cs index 8f1cc38e..3a91a606 100644 --- a/tests/NodePilot.Api.Tests/Services/DbAdmin/DbAdminReadOnlySqlGuardTests.cs +++ b/tests/NodePilot.Api.Tests/Services/DbAdmin/DbAdminReadOnlySqlGuardTests.cs @@ -12,6 +12,28 @@ namespace NodePilot.Api.Tests.Services.DbAdmin; /// public class DbAdminReadOnlySqlGuardTests { + [Theory] + [InlineData("SELECT E'hello'")] + [InlineData("SELECT e'hello'")] + [InlineData("""SELECT E'\'' AS marker, "PasswordHash" AS harmless FROM "Users" -- '""")] + [InlineData("""WITH data AS (SELECT e'\'' AS marker, "PasswordHash" AS harmless FROM "Users" -- '""" + "\n) SELECT harmless FROM data")] + public void Validate_RejectsUnsupportedEscapeStringSyntax(string sql) + { + var act = () => DbAdminReadOnlySqlGuard.Validate(sql); + act.Should().Throw().WithMessage("*Escape-string literals are not allowed*"); + } + + [Theory] + [InlineData("SELECT 'Dianne''s horse'")] + [InlineData("SELECT 'E''hello''' AS text")] + [InlineData("SELECT $$ E'hello' $$ AS text")] + [InlineData("SELECT $tag$ E'hello' $tag$ AS text")] + [InlineData("SELECT 1 /* E'hello' */")] + [InlineData("SELECT 1 -- E'hello'")] + [InlineData("SELECT \"E'hello'\" FROM Workflows")] + public void Validate_OrdinaryLiteralsAndQuotedEscapeText_RemainAllowed(string sql) + => FluentActions.Invoking(() => DbAdminReadOnlySqlGuard.Validate(sql)).Should().NotThrow(); + [Theory] [InlineData("SELECT * FROM Users")] [InlineData("SELECT u.* FROM Users u")]