diff --git a/src/SQLInsight.Core/CustomRules/CustomRuleDefinition.cs b/src/SQLInsight.Core/CustomRules/CustomRuleDefinition.cs index 69bac89..49793b5 100644 --- a/src/SQLInsight.Core/CustomRules/CustomRuleDefinition.cs +++ b/src/SQLInsight.Core/CustomRules/CustomRuleDefinition.cs @@ -112,28 +112,10 @@ public sealed partial record CustomRuleDefinition } /// - /// The query must be a single read-only SELECT. This is a second line of defence: the agent's login has no write - /// permissions to begin with (grant-least-privilege.sql). + /// The query must be a single read-only SELECT (). This is a second line of defence: the + /// agent's login has no write permissions to begin with (grant-least-privilege.sql). /// - public static IEnumerable QueryProblems(string query) - { - ArgumentNullException.ThrowIfNull(query); - var code = StringsAndComments().Replace(query, " "); - if (!StartsWithSelect().IsMatch(code)) - { - yield return "Forespørgslen skal starte med SELECT eller WITH."; - } - - foreach (Match forbidden in ForbiddenKeywords().Matches(code)) - { - yield return $"Forespørgslen må ikke indeholde {forbidden.Value.ToUpperInvariant()}: regler må kun læse."; - } - - if (code.TrimEnd().TrimEnd(';').Contains(';', StringComparison.Ordinal)) - { - yield return "Forespørgslen må kun være én sætning (intet semikolon undervejs)."; - } - } + public static IEnumerable QueryProblems(string query) => ReadOnlyQuery.Problems(query); /// Replaces {column} with the row's value; unknown placeholders are left as they are. public static string Fill(string template, IReadOnlyDictionary row) @@ -146,15 +128,6 @@ public sealed partial record CustomRuleDefinition [GeneratedRegex(@"^(COM|LOCAL)-[A-Z]{2,10}-[0-9]{3}$")] private static partial Regex IdPattern(); - [GeneratedRegex(@"'(?:[^']|'')*'|\[[^\]]*\]|""[^""]*""|--[^\n]*|/\*.*?\*/", RegexOptions.Singleline)] - private static partial Regex StringsAndComments(); - - [GeneratedRegex(@"^\s*(SELECT|WITH)\b", RegexOptions.IgnoreCase)] - private static partial Regex StartsWithSelect(); - - [GeneratedRegex(@"\b(INSERT|UPDATE|DELETE|MERGE|DROP|ALTER|CREATE|TRUNCATE|EXEC|EXECUTE|GRANT|REVOKE|DENY|SHUTDOWN|KILL|BACKUP|RESTORE|DBCC|RECONFIGURE|OPENROWSET|OPENDATASOURCE|OPENQUERY|INTO|WAITFOR|USE|BULK|SETUSER)\b|\b(sp|xp)_\w+", RegexOptions.IgnoreCase)] - private static partial Regex ForbiddenKeywords(); - [GeneratedRegex(@"\{([A-Za-z0-9_ ]+)\}")] private static partial Regex Placeholder(); } diff --git a/src/SQLInsight.Core/CustomRules/ReadOnlyQuery.cs b/src/SQLInsight.Core/CustomRules/ReadOnlyQuery.cs new file mode 100644 index 0000000..dfd7301 --- /dev/null +++ b/src/SQLInsight.Core/CustomRules/ReadOnlyQuery.cs @@ -0,0 +1,186 @@ +using System.Text; +using System.Text.RegularExpressions; + +namespace SQLInsight.Core.CustomRules; + +/// +/// Whether a query someone typed (a custom rule, a synthetic SQL check) is one read-only SELECT. T-SQL is lexed the +/// way SQL Server reads it: nested block comments, '' in strings, ]] in [identifiers] and "" in +/// "identifiers", so a keyword cannot hide in what only looks like a comment or a string. T-SQL needs no semicolon +/// between statements, so besides the writing keywords every statement keyword is refused, and a second SELECT is +/// only accepted after UNION, EXCEPT or INTERSECT. +/// This is a second line of defence: the login has no write permissions to begin with (grant-least-privilege.sql), and +/// a synthetic check also runs the query in a transaction that is always rolled back. +/// +public static partial class ReadOnlyQuery +{ + public static IEnumerable Problems(string query) + { + ArgumentNullException.ThrowIfNull(query); + var (code, unterminated) = Code(query); + if (unterminated is not null) + { + yield return $"Forespørgslen har {unterminated}, der ikke er afsluttet."; + yield break; + } + + if (!StartsWithSelect().IsMatch(code)) + { + yield return "Forespørgslen skal starte med SELECT eller WITH."; + } + + foreach (var forbidden in ForbiddenKeywords().Matches(code).Select(m => Regex.Replace(m.Value.ToUpperInvariant(), @"\s+", " ")).Distinct(StringComparer.Ordinal)) + { + yield return $"Forespørgslen må ikke indeholde {forbidden}: forespørgslen må kun læse (sæt [ ] om et kolonnenavn, der hedder sådan)."; + } + + if (code.TrimEnd().TrimEnd(';').Contains(';', StringComparison.Ordinal) || !OneStatement(code)) + { + yield return "Forespørgslen må kun være én sætning (intet semikolon undervejs, og flere SELECT kun med UNION, EXCEPT eller INTERSECT)."; + } + } + + /// + /// The query with comments, strings and quoted identifiers replaced by a space, and what was left open (a comment, + /// a string, an identifier) or null. + /// + internal static (string Code, string? Unterminated) Code(string query) + { + var code = new StringBuilder(query.Length); + var i = 0; + while (i < query.Length) + { + var c = query[i]; + var next = i + 1 < query.Length ? query[i + 1] : '\0'; + if (c == '-' && next == '-') + { + var end = query.IndexOf('\n', i); + i = end < 0 ? query.Length : end; + code.Append(' '); + } + else if (c == '/' && next == '*') + { + // SQL Server nests block comments: /* /* */ is still inside the comment. + var depth = 0; + do + { + if (i + 1 < query.Length && query[i] == '/' && query[i + 1] == '*') + { + depth++; + i += 2; + } + else if (i + 1 < query.Length && query[i] == '*' && query[i + 1] == '/') + { + depth--; + i += 2; + } + else + { + i++; + } + } + while (depth > 0 && i < query.Length); + + if (depth > 0) + { + return (code.ToString(), "en kommentar (/* … */)"); + } + + code.Append(' '); + } + else if (c is '\'' or '[' or '"') + { + var close = c == '[' ? ']' : c; + var j = i + 1; + while (true) + { + j = query.IndexOf(close, j); + if (j < 0) + { + return (code.ToString(), c == '\'' ? "en tekst ('…')" : "et navn i " + c + close); + } + + if (j + 1 < query.Length && query[j + 1] == close) + { + j += 2; // '' or ]] or "": the quote itself, still inside + continue; + } + + break; + } + + i = j + 1; + code.Append(' '); + } + else + { + code.Append(c); + i++; + } + } + + return (code.ToString(), null); + } + + /// + /// One statement: the SELECTs outside parentheses are the query's own SELECT (for WITH, the one right after the + /// common table expressions) and those joined to it by UNION, EXCEPT or INTERSECT. "SELECT 1 SELECT 2" is two. + /// + private static bool OneStatement(string code) + { + var depth = 0; + var mainSeen = false; + var startsWithWith = StartsWithWith().IsMatch(code); + string? previous = null; + foreach (Match token in Tokens().Matches(code)) + { + var value = token.Value; + if (value == "(") + { + depth++; + } + else if (value == ")") + { + depth = Math.Max(0, depth - 1); + } + else if (depth == 0 && value.Equals("SELECT", StringComparison.OrdinalIgnoreCase)) + { + var joined = previous is not null && (previous.Equals("UNION", StringComparison.OrdinalIgnoreCase) || previous.Equals("ALL", StringComparison.OrdinalIgnoreCase) + || previous.Equals("EXCEPT", StringComparison.OrdinalIgnoreCase) || previous.Equals("INTERSECT", StringComparison.OrdinalIgnoreCase)); + var first = !mainSeen && (startsWithWith ? previous == ")" : previous is null); + if (!joined && !first) + { + return false; + } + + mainSeen = true; + } + + previous = value; + } + + return true; + } + + [GeneratedRegex(@"^\s*(SELECT|WITH)\b", RegexOptions.IgnoreCase)] + private static partial Regex StartsWithSelect(); + + [GeneratedRegex(@"^\s*WITH\b", RegexOptions.IgnoreCase)] + private static partial Regex StartsWithWith(); + + [GeneratedRegex(@"[()]|[A-Za-z_@#][A-Za-z0-9_@#$]*|[^\sA-Za-z_@#()]")] + private static partial Regex Tokens(); + + /// + /// Writing keywords, every other statement keyword (T-SQL needs no semicolon between statements), sequences + /// (NEXT VALUE FOR is not rolled back) and table hints that take update or exclusive locks. + /// + [GeneratedRegex( + @"\b(INSERT|UPDATE|DELETE|MERGE|DROP|ALTER|CREATE|TRUNCATE|EXEC|EXECUTE|GRANT|REVOKE|DENY|SHUTDOWN|KILL|BACKUP|RESTORE|DBCC|RECONFIGURE" + + @"|OPENROWSET|OPENDATASOURCE|OPENQUERY|INTO|WAITFOR|USE|BULK|SETUSER" + + @"|WRITETEXT|UPDATETEXT|READTEXT|CHECKPOINT|COMMIT|ROLLBACK|SAVE|BEGIN|TRAN|TRANSACTION|DECLARE|SET|PRINT|RAISERROR|THROW|IF|WHILE|GOTO|RETURN" + + @"|BREAK|CONTINUE|OPEN|CLOSE|DEALLOCATE|REVERT|ADD|ENABLE|DISABLE|RECEIVE|SEND|CONVERSATION|DIALOG|LOAD|DUMP" + + @"|UPDLOCK|XLOCK|TABLOCK|TABLOCKX|HOLDLOCK|PAGLOCK|SERIALIZABLE|REPEATABLEREAD)\b|\bNEXT\s+VALUE\s+FOR\b|\b(sp|xp)_\w+", + RegexOptions.IgnoreCase)] + private static partial Regex ForbiddenKeywords(); +} diff --git a/tests/SQLInsight.Core.Tests/ReadOnlyQueryTests.cs b/tests/SQLInsight.Core.Tests/ReadOnlyQueryTests.cs new file mode 100644 index 0000000..bd3193c --- /dev/null +++ b/tests/SQLInsight.Core.Tests/ReadOnlyQueryTests.cs @@ -0,0 +1,55 @@ +using SQLInsight.Core.CustomRules; + +namespace SQLInsight.Core.Tests; + +/// A typed query (custom rule, synthetic SQL check) is one read-only SELECT, also when someone tries to hide more. +public class ReadOnlyQueryTests +{ + [Theory] + // A ]] in an [identifier] is not its end: the ' after it is no string, and the DELETE is code. + [InlineData("SELECT 1 AS [x]]'] ; DELETE FROM dbo.t; SELECT 'a'")] + [InlineData("SELECT 1 AS [x]]'] DELETE FROM dbo.t SELECT 'a'")] + // Block comments nest in T-SQL; an unterminated one is refused. + [InlineData("SELECT 1 /* /* */ DELETE FROM dbo.t")] + [InlineData("SELECT 1 AS \"a\"\"'\" DELETE FROM dbo.t --'")] + // No semicolon is needed between statements. + [InlineData("SELECT 1 COMMIT")] + [InlineData("SELECT 1 SELECT 2")] + [InlineData("SELECT 1 WRITETEXT dbo.t.c @p 'x'")] + [InlineData("SELECT 1 UPDATETEXT dbo.t.c @p 0 NULL 'x'")] + [InlineData("SELECT 1 DECLARE @x int")] + [InlineData("SELECT 1 SET CONTEXT_INFO 0x01")] + [InlineData("SELECT 1 RAISERROR('x', 16, 1) WITH LOG")] + [InlineData("SELECT 1 CHECKPOINT")] + [InlineData("SELECT 1 ROLLBACK")] + [InlineData("SELECT 1 BEGIN TRAN")] + [InlineData("SELECT 1 ENABLE TRIGGER trg ON dbo.t")] + [InlineData("SELECT 1 WHILE 1 = 1 SELECT 2")] + [InlineData("WITH q AS (SELECT 1 AS a) SELECT a FROM q SELECT 2")] + [InlineData("SELECT 1 x RECEIVE TOP (1) * FROM dbo.q")] + [InlineData("SELECT 1 ADD SIGNATURE TO dbo.p BY CERTIFICATE c")] + // Side effects inside one SELECT: sequences are not rolled back; update and exclusive locks block production. + [InlineData("SELECT NEXT VALUE FOR dbo.InvoiceNumbers")] + [InlineData("SELECT * FROM dbo.CustTable WITH (UPDLOCK, HOLDLOCK)")] + [InlineData("SELECT * FROM dbo.CustTable WITH (TABLOCKX)")] + [InlineData("SELECT * FROM dbo.CustTable WITH (XLOCK)")] + [InlineData("SELECT * INTO dbo.Copy FROM dbo.CustTable")] + [InlineData("SELECT 1 EXEC sp_who")] + [InlineData("SELECT * FROM OPENQUERY(LINKED, 'EXEC x')")] + [InlineData("select 1 exec('delete dbo.t')")] + [InlineData("SELECT 'unterminated")] + [InlineData("SELECT [unterminated")] + public void Hidden_or_extra_statements_are_refused(string query) => Assert.NotEmpty(ReadOnlyQuery.Problems(query)); + + [Theory] + [InlineData("SELECT 1")] + [InlineData("SELECT COUNT(*) FROM dbo.DATAAREA")] + [InlineData("SELECT [update], [into], [x]]y] FROM dbo.audit")] + [InlineData("SELECT 'DELETE; EXEC ''x''' AS text /* INSERT /* nested */ COMMIT */ -- DROP")] + [InlineData("WITH d AS (SELECT name FROM sys.databases), e (n) AS (SELECT 1) SELECT * FROM d;")] + [InlineData("SELECT name FROM sys.databases UNION ALL SELECT name FROM sys.objects EXCEPT SELECT N'x' INTERSECT SELECT N'x'")] + [InlineData("SELECT TOP (10) a FROM (SELECT 1 AS a) x WHERE a IN (SELECT 1) ORDER BY a OFFSET 0 ROWS FETCH NEXT 5 ROWS ONLY")] + [InlineData("SELECT * FROM dbo.CustTable WITH (NOLOCK) OPTION (MAXDOP 1)")] + [InlineData("SELECT IIF(1 = 1, GETDATE(), NULL), \"quoted\"\"name\" FROM sys.objects")] + public void Read_only_queries_are_accepted(string query) => Assert.Empty(ReadOnlyQuery.Problems(query)); +}