From a55b7e51ca8fe0fe2359c1d11c31e2691e60beb9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 15:13:32 -0400 Subject: [PATCH] =?UTF-8?q?test(sql):=20injection=20regression=20=E2=80=94?= =?UTF-8?q?=20bind=20values,=20reject=20unknown=20identifiers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Locks the driver's injection guarantee against the SQLite fixture. An authored VALUE ('; DROP TABLE TagValues; --) binds as a DbParameter and can only ever be a key that matches no row (BadNoData); the seed table survives. A hostile IDENTIFIER in table/column position is dialect-quoted into a single nonexistent identifier — inert — and the payload never executes. Scope, stated plainly: design §8.1 also specifies a catalog gate (validate an authored identifier against INFORMATION_SCHEMA, reject an unknown one as BadNodeIdUnknown). That gate does NOT exist in the driver yet and no task here builds it, so a hostile identifier is not rejected up front — it is quoted, the query fails after the connection opened, and the tag Bad-codes as a query failure (BadCommunicationError), not BadNodeIdUnknown. These tests assert what the code actually guarantees — the payload is inert and the table intact — rather than a catalog gate that isn't there. The gate is a tracked follow-up. No implementation change: the reader already binds correctly (Task 7); this suite pins it. Falsifiability-checked — forcing a real DROP before the row-count assertion turns it red, so "table survives" is load-bearing. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- .../SqlInjectionRegressionTests.cs | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs new file mode 100644 index 00000000..784b3dd7 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs @@ -0,0 +1,168 @@ +using Microsoft.Data.Sqlite; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +/// +/// Locks the driver's SQL-injection guarantee against a real database (): +/// an authored value is bound as a parameter and can never execute, and an authored +/// identifier — which cannot be parameterized in SQL — is dialect-quoted so a hostile one becomes +/// an inert reference to a nonexistent object rather than executable text. If these pass, a malicious tag +/// cannot alter the database; the seed table is intact after every hostile poll. +/// Scope note — what this suite does NOT assume. Design §8.1 also specifies a catalog gate: +/// validate every authored table/column against INFORMATION_SCHEMA before quoting it, so an unknown +/// identifier rejects the tag. No such gate exists in the driver yet (see the "Not yet +/// implemented" note on ), and no task in this workstream builds one. So a hostile +/// identifier here is not rejected as BadNodeIdUnknown — it is bracket-/double-quoted into a +/// single nonexistent identifier, the query then fails after the connection opened, and the tag Bad-codes +/// as a query failure (). This suite proves the payload +/// is inert — it does not execute and the table survives — which is the guarantee the code +/// actually makes. The catalog gate is a separate follow-up; a test that pretended it existed would be +/// asserting fiction. +/// +public sealed class SqlInjectionRegressionTests +{ + private static readonly TimeSpan CommandTimeout = TimeSpan.FromSeconds(10); + private static readonly TimeSpan OperationTimeout = TimeSpan.FromSeconds(15); + + /// The classic payload: were the key concatenated into text, this would drop the table. + private const string DropTablePayload = "'; DROP TABLE TagValues; --"; + + [Fact] + public async Task MaliciousKeyValue_bindsHarmlessly_tableSurvives() + { + using var fixture = new SqlitePollFixture(); + var reader = NewReader(fixture, KvTag("Speed", DropTablePayload)); + + var snapshots = await reader.ReadAsync(["Speed"], CancellationToken.None); + + // Bound, not executed: it is simply a key that matches no row. + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadNoData); + // The table is still there, with its seeded rows — the DROP never ran. + (await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable)).ShouldBeGreaterThan(0); + } + + [Fact] + public async Task MaliciousKeyValue_doesNotEvenDisturbAHealthyNeighbourOnTheSamePoll() + { + // A hostile key in one slot must not corrupt a legitimate tag sharing the poll — it binds as its own + // parameter and matches nothing; the real key still reads. + using var fixture = new SqlitePollFixture(); + var reader = NewReader(fixture, + KvTag("Evil", DropTablePayload), + KvTag("Speed", SqlitePollFixture.PresentKey)); + + var snapshots = await reader.ReadAsync(["Evil", "Speed"], CancellationToken.None); + + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadNoData); + snapshots[1].Value.ShouldBe(SqlitePollFixture.PresentValue); + (await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable)).ShouldBeGreaterThan(0); + } + + [Fact] + public async Task MaliciousTableIdentifier_isInert_neverExecutesAndTheSeedSurvives() + { + using var fixture = new SqlitePollFixture(); + var seedRows = await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable); + + // A table name carrying a statement terminator + DROP. There is NO catalog gate, so this is not + // rejected up front — it is quoted into one nonexistent identifier. The query fails (no such table), + // the connection having opened, so the tag Bad-codes as a query failure. The DROP never runs. + var reader = NewReader(fixture, + new SqlTagDefinition( + Name: "Evil", + Model: SqlTagModel.KeyValue, + Table: "TagValues\"; DROP TABLE TagValues; --", + KeyColumn: SqlitePollFixture.KeyColumn, + KeyValue: SqlitePollFixture.PresentKey, + ValueColumn: SqlitePollFixture.ValueColumn, + TimestampColumn: SqlitePollFixture.TimestampColumn)); + + var snapshots = await reader.ReadAsync(["Evil"], CancellationToken.None); + + // Bad-coded — but as a QUERY failure, not BadNodeIdUnknown (there is no identifier-catalog gate). + SqlStatusCodes.IsBad(snapshots[0].StatusCode).ShouldBeTrue(); + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadCommunicationError); + // The load-bearing assertion: the payload did NOT execute. The table and its rows are untouched. + (await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable)).ShouldBe(seedRows); + } + + [Fact] + public async Task MaliciousColumnIdentifier_isInert_neverExecutesAndTheSeedSurvives() + { + using var fixture = new SqlitePollFixture(); + var seedRows = await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable); + + // The same attack via the value column: quoted, so it becomes a nonexistent column reference; the + // SELECT fails and the tag Bad-codes. Nothing executes. + var reader = NewReader(fixture, + new SqlTagDefinition( + Name: "Evil", + Model: SqlTagModel.KeyValue, + Table: SqlitePollFixture.KeyValueTable, + KeyColumn: SqlitePollFixture.KeyColumn, + KeyValue: SqlitePollFixture.PresentKey, + ValueColumn: "num_value\"; DROP TABLE TagValues; --", + TimestampColumn: SqlitePollFixture.TimestampColumn)); + + var snapshots = await reader.ReadAsync(["Evil"], CancellationToken.None); + + SqlStatusCodes.IsBad(snapshots[0].StatusCode).ShouldBeTrue(); + (await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable)).ShouldBe(seedRows); + } + + [Fact] + public async Task AControlCharacterIdentifier_isRejectedByQuoting_beforeItReachesTheDatabase() + { + // QuoteIdentifier refuses a NUL/control-character identifier outright (it cannot name a real object). + // The planner throws ArgumentException, which the reader classes as an authoring fault — + // BadConfigurationError — rather than a database failure. Still inert: the table survives. + using var fixture = new SqlitePollFixture(); + var seedRows = await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable); + + var reader = NewReader(fixture, + new SqlTagDefinition( + Name: "Evil", + Model: SqlTagModel.KeyValue, + Table: "Tag\0Values", + KeyColumn: SqlitePollFixture.KeyColumn, + KeyValue: SqlitePollFixture.PresentKey, + ValueColumn: SqlitePollFixture.ValueColumn, + TimestampColumn: SqlitePollFixture.TimestampColumn)); + + var snapshots = await reader.ReadAsync(["Evil"], CancellationToken.None); + + snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadConfigurationError); + (await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable)).ShouldBe(seedRows); + } + + // ---- helpers ---- + + private static SqlPollReader NewReader(SqlitePollFixture fixture, params SqlTagDefinition[] tags) + { + var table = tags.ToDictionary(t => t.Name, StringComparer.Ordinal); + return new SqlPollReader( + fixture.Factory, fixture.ConnectionString, new SqliteDialect(), + commandTimeout: CommandTimeout, operationTimeout: OperationTimeout, + maxConcurrentGroups: 4, nullIsBad: false, + resolve: rawPath => table.GetValueOrDefault(rawPath)); + } + + private static SqlTagDefinition KvTag(string name, string keyValue) + => new(name, SqlTagModel.KeyValue, SqlitePollFixture.KeyValueTable, + KeyColumn: SqlitePollFixture.KeyColumn, KeyValue: keyValue, + ValueColumn: SqlitePollFixture.ValueColumn, + TimestampColumn: SqlitePollFixture.TimestampColumn); + + /// Counts rows in a table over the fixture's own connection — the direct evidence a DROP did not + /// run. The table name is a test constant, never authored input, so interpolating it here is safe. + private static async Task RowCountAsync(SqlitePollFixture fixture, string table) + { + await using var command = fixture.Connection.CreateCommand(); + command.CommandText = $"SELECT COUNT(*) FROM \"{table}\""; + return Convert.ToInt64(await command.ExecuteScalarAsync()); + } +}