50f88b938f
Three findings from the code review of the dialect seam:
- QuoteIdentifier used char.IsControl, which only covers Unicode category Cc.
Bidi overrides, zero-width spaces, soft hyphen and BOM are category Cf and
passed through untouched. They cannot escape the brackets, so this is not an
injection bypass — but the method rejects control characters precisely to
protect the integrity of logged/audited statements, and Cf characters spoof
rendered text while comparing byte-different from the real catalog name
(Trojan Source, CVE-2021-42574). Same rule, same rationale, wider category.
- The XML docs on both files asserted that identifiers 'are sourced only from
catalog-validated (INFORMATION_SCHEMA) names' and that rejection is 'a
backstop, not the primary defence'. Neither is true yet: design §8.1 does
specify that gate, but nothing implements it and no task in the plan
schedules one, so an authored TagConfig table/column reaches QuoteIdentifier
unfiltered. The docs now say so, because under-scrutinising this method on
the strength of a filter that does not exist is exactly the failure mode.
- The control-character test only pinned NUL, so a regression to a
Contains('\0') check would still have passed. Pinned the whole Cc category
plus the new Cf rule; verified load-bearing by deleting the guard and
observing exactly the 5 new cases go red.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
144 lines
6.2 KiB
C#
144 lines
6.2 KiB
C#
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;
|
|
|
|
/// <summary>
|
|
/// Golden tests for the driver's SQL-injection boundary. Values are always bound as
|
|
/// <c>DbParameter</c>s; identifiers cannot be, so <see cref="SqlServerDialect.QuoteIdentifier"/> is the
|
|
/// only thing standing between a catalog-sourced name and a command text. These tests pin the escape
|
|
/// rule, the rejection rules, and the exact catalog SQL.
|
|
/// </summary>
|
|
public class SqlServerDialectTests
|
|
{
|
|
[Fact]
|
|
public void QuoteIdentifier_bracketsAndEscapesEmbeddedBrackets()
|
|
{
|
|
var d = new SqlServerDialect();
|
|
d.QuoteIdentifier("Speed").ShouldBe("[Speed]");
|
|
d.QuoteIdentifier("a]b").ShouldBe("[a]]b]"); // ] doubled per T-SQL rules
|
|
}
|
|
|
|
[Fact]
|
|
public void QuoteIdentifier_rejectsControlCharsAndNul()
|
|
=> Should.Throw<ArgumentException>(() => new SqlServerDialect().QuoteIdentifier("x\0y"));
|
|
|
|
// NUL alone would still pass against a `Contains('\0')` regression, so pin the whole Cc category.
|
|
[Theory]
|
|
[InlineData("x\u0001y")] // C0 SOH
|
|
[InlineData("x\u001By")] // C0 ESC
|
|
[InlineData("x\u007Fy")] // DEL
|
|
[InlineData("x\u0085y")] // C1 NEL
|
|
[InlineData("x\ty")]
|
|
[InlineData("x\ny")]
|
|
public void QuoteIdentifier_rejectsEveryControlCharacter_notJustNul(string ident)
|
|
=> Should.Throw<ArgumentException>(() => new SqlServerDialect().QuoteIdentifier(ident));
|
|
|
|
// Unicode Format (Cf) is a distinct category from Control (Cc): char.IsControl returns false for all
|
|
// of these. They cannot escape the brackets, but they spoof rendered log/AdminUI text while comparing
|
|
// byte-different from the real catalog name (Trojan Source, CVE-2021-42574).
|
|
[Theory]
|
|
[InlineData("Sp\u200Beed")] // zero-width space
|
|
[InlineData("\u202Ediop.selbat")] // right-to-left override
|
|
[InlineData("x\u2066y\u2069")] // bidi isolates
|
|
[InlineData("x\uFEFFy")] // BOM / zero-width no-break space
|
|
[InlineData("x\u00ADy")] // soft hyphen
|
|
public void QuoteIdentifier_rejectsUnicodeFormatCharacters(string ident)
|
|
=> Should.Throw<ArgumentException>(() => new SqlServerDialect().QuoteIdentifier(ident));
|
|
|
|
[Theory]
|
|
[InlineData("bit", DriverDataType.Boolean)]
|
|
[InlineData("int", DriverDataType.Int32)]
|
|
[InlineData("bigint", DriverDataType.Int64)]
|
|
[InlineData("real", DriverDataType.Float32)]
|
|
[InlineData("float", DriverDataType.Float64)]
|
|
[InlineData("decimal", DriverDataType.Float64)]
|
|
[InlineData("nvarchar", DriverDataType.String)]
|
|
[InlineData("datetime2", DriverDataType.DateTime)]
|
|
[InlineData("uniqueidentifier", DriverDataType.String)]
|
|
public void MapColumnType_mapsFamilies(string sql, DriverDataType expected)
|
|
=> new SqlServerDialect().MapColumnType(sql).ShouldBe(expected);
|
|
|
|
[Fact]
|
|
public void CatalogSql_isInformationSchemaAndParameterized()
|
|
{
|
|
var d = new SqlServerDialect();
|
|
d.LivenessSql.ShouldBe("SELECT 1");
|
|
d.ListTablesSql.ShouldContain("INFORMATION_SCHEMA.TABLES");
|
|
d.ListTablesSql.ShouldContain("@schema"); // never string-interpolated
|
|
d.ListColumnsSql.ShouldContain("@table");
|
|
}
|
|
|
|
[Fact]
|
|
public void SingleRowLimit_isTSqlsTop1_inThePrefixPosition_withItsOwnTrailingSpace()
|
|
{
|
|
var d = new SqlServerDialect();
|
|
|
|
// The planner concatenates "SELECT " + prefix + columns with no separator of its own, so the
|
|
// trailing space belongs to the fragment. T-SQL has no end-of-statement limit clause.
|
|
d.SingleRowLimitPrefix.ShouldBe("TOP 1 ");
|
|
d.SingleRowLimitSuffix.ShouldBe("");
|
|
string.Concat("SELECT ", d.SingleRowLimitPrefix, "[a]", d.SingleRowLimitSuffix)
|
|
.ShouldBe("SELECT TOP 1 [a]");
|
|
}
|
|
|
|
// ---- extra guards on the injection boundary (beyond the plan's golden set) ----
|
|
|
|
[Fact]
|
|
public void Dialect_identifiesItselfAsSqlServer_andExposesTheAbstractFactory()
|
|
{
|
|
var d = new SqlServerDialect();
|
|
d.Provider.ShouldBe(SqlProvider.SqlServer);
|
|
d.Factory.ShouldNotBeNull();
|
|
// The seam must not leak Microsoft.Data.SqlClient into its signature.
|
|
typeof(ISqlDialect).GetProperty(nameof(ISqlDialect.Factory))!
|
|
.PropertyType.ShouldBe(typeof(System.Data.Common.DbProviderFactory));
|
|
}
|
|
|
|
[Theory]
|
|
[InlineData(null)]
|
|
[InlineData("")]
|
|
[InlineData(" ")]
|
|
[InlineData("\t")]
|
|
public void QuoteIdentifier_rejectsNullEmptyAndWhitespace(string? ident)
|
|
=> Should.Throw<ArgumentException>(() => new SqlServerDialect().QuoteIdentifier(ident!));
|
|
|
|
[Fact]
|
|
public void QuoteIdentifier_escapesAnIdentifierThatIsNothingButABracket()
|
|
=> new SqlServerDialect().QuoteIdentifier("]").ShouldBe("[]]]"); // T-SQL for the 1-char name "]"
|
|
|
|
[Fact]
|
|
public void QuoteIdentifier_acceptsExactly128Chars_andRejects129()
|
|
{
|
|
var d = new SqlServerDialect();
|
|
d.QuoteIdentifier(new string('a', 128)).ShouldBe("[" + new string('a', 128) + "]");
|
|
Should.Throw<ArgumentException>(() => d.QuoteIdentifier(new string('a', 129)));
|
|
}
|
|
|
|
[Fact]
|
|
public void QuoteIdentifier_neutralisesAClassicInjectionPayload()
|
|
{
|
|
// A hostile "column name" cannot escape the brackets: the only metacharacter that could is ],
|
|
// and it is doubled. The result is a single (nonexistent) identifier, never executable SQL.
|
|
new SqlServerDialect().QuoteIdentifier("x] ; DROP TABLE Users --")
|
|
.ShouldBe("[x]] ; DROP TABLE Users --]");
|
|
}
|
|
|
|
[Theory]
|
|
[InlineData("NVARCHAR", DriverDataType.String)]
|
|
[InlineData("BiT", DriverDataType.Boolean)]
|
|
public void MapColumnType_isCaseInsensitive(string sql, DriverDataType expected)
|
|
=> new SqlServerDialect().MapColumnType(sql).ShouldBe(expected);
|
|
|
|
[Theory]
|
|
[InlineData("geography")]
|
|
[InlineData("sql_variant")]
|
|
[InlineData("timestamp")] // SQL Server rowversion — binary, deliberately NOT a DateTime
|
|
[InlineData("")]
|
|
[InlineData(null)]
|
|
public void MapColumnType_unknownFamilyFallsBackToString_andNeverThrows(string? sql)
|
|
=> new SqlServerDialect().MapColumnType(sql!).ShouldBe(DriverDataType.String);
|
|
}
|