Add TDS token data length bounds checks

[paulmedynski]

Summary

Adds bounds checks on data lengths read from the TDS wire to prevent unbounded byte[] allocation from a malicious server.

All checks throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, ...) when a spoofed length exceeds protocol-reasonable maximums.

Changes

File	Change	Description	Tests
TdsParser.cs L4180	TryProcessSessionState — total length check	Rejects tokenLen > MaxTokenDataLength (1 MB)	SessionState_OversizedTotalLength
TdsParser.cs L4233	TryProcessSessionState — inner option length check	Rejects individual state option with stateLen < 0 or stateLen > MaxTokenDataLength	SessionState_OversizedInnerOptionLength, SessionState_NegativeInnerOptionLength
TdsParser.cs L4454	TryProcessFedAuthInfo — token length check	Rejects tokenLen > MaxTokenDataLength	FedAuthInfo_OversizedTokenLength
TdsParser.cs L3351	TryProcessEnvChange — PromoteTransaction newLength check	Rejects newLength > MaxPromoteTransactionLength (64 KB)	EnvChange_PromoteTransaction_OversizedLength (login), BatchResponse_PromoteTransaction_OversizedLength (post-login)
TdsParser.cs L3853	TryProcessFeatureExtAck — data length check	Rejects dataLen > MaxTokenDataLength	FeatureExtAck_OversizedDataLength, FeatureExtAck_MaxAllowedDataLength_DoesNotThrow
TdsParser.cs L7144	TryReadSqlDateTime — data length check	Rejects length > MaxDateTimeLength (10)	BatchResponse_DateTime_OversizedLength
TdsParser.cs L4937-4948	TryProcessReturnValue — non-PLP length check	Restructured: IsPlp types skip bounds check; non-PLP rejects valLen > int.MaxValue	BatchResponse_ReturnValue_OversizedLength, BatchResponse_ReturnValue_PlpUnknownLength
TdsParser.cs L7405	TryReadSqlValueInternal — binary/vector length check	Rejects length < 0 or length > MAXSIZE (8000) for binary types in sql_variant path	BatchResponse_SqlVariantBinary_OversizedLength, BatchResponse_SqlVariantBinary_NegativeLength
SqlConnectionInternal.cs L1349	OnFeatureExtAck — SRECOVERY inner state length	Validates inner state option length doesn't exceed remaining buffer	SRecovery_MalformedInnerStateLength
TdsEnums.cs L90-96	Constants	MaxTokenDataLength (1 MB), MaxPromoteTransactionLength (64 KB), MaxDateTimeLength (10)	(used by all above)

Impact analysis

Group 1: New upper-bound constraints

These add hard limits where previously the parser would attempt to allocate whatever the server claimed. A conforming SQL Server will never exceed any of these limits, but a misbehaving proxy, network appliance, or third-party TDS implementation could.

Location	Limit	Rationale
FeatureExtAck dataLen	1 MB	No legitimate feature ack payload approaches this
SessionState total length	1 MB	Session state tokens are typically single-digit KB
SessionState inner option	≥ 0 and ≤ 1 MB	Individual state options are even smaller; negative values indicate wire corruption
FedAuthInfo tokenLen	1 MB	Contains only STS URL + SPN — a few hundred bytes
PromoteTransaction newLength	64 KB	DTC propagation tokens are typically ~200–500 bytes
DateTime length	10 bytes	DateTimeOffset (the largest) is exactly 10; this is the protocol ceiling
Binary/vector in sql_variant	≥ 0 and ≤ 8000	MAXSIZE is the protocol maximum for non-PLP binary; negative lenData results from a total variant length smaller than the type overhead

Risk to existing apps: Effectively zero against real SQL Server. Could break connections through a non-conforming TDS proxy that inflates token lengths (e.g., for padding or encapsulation).

Group 2: Debug.Assert → runtime exception (bug fix)

These replaced Debug.Assert (which is stripped in Release builds, meaning the invalid state was silently accepted) with a throwing check:

Location	Previous behavior (Release)	New behavior
ReturnValue non-PLP valLen > int.MaxValue	Silently truncated to int.MaxValue	Throws ParsingErrorLength
Binary/Vector in sql_variant length > MAXSIZE	No check at all (assert stripped)	Throws ParsingErrorLength

Risk to existing apps: Only if the app was previously "surviving" corrupt data by silently reading garbage. In practice these indicate a corrupted stream that would likely cause downstream failures anyway.

Group 3: Buffer overrun prevention (defense-in-depth)

Location	Issue prevented
SqlConnectionInternal.OnFeatureExtAck SRECOVERY: len < 0 || len > data.Length - i	Out-of-bounds Buffer.BlockCopy from a malformed feature ack payload

Risk to existing apps: None — this path only fires if the server sends internally inconsistent SRECOVERY data (claimed inner length exceeds the outer buffer already validated by the FeatureExtAck check).

Group 4: Behavioral improvement (non-breaking)

Location	Change
TryReadSqlDateTime	Replaced ((uint)length <= 16) ? stackalloc : new byte[length] with unconditional stackalloc byte[10]

Since the bounds check guarantees length ≤ 10, the conditional heap allocation path is dead code eliminated. Minor perf win. No user-visible behavior change.

Impact summary

Category	Count	Risk to legitimate apps
New constraints	6 sites	~Zero (limits far exceed protocol maximums)
Assert → exception	2 sites	Near-zero (previously silent corruption)
Buffer overrun fix	1 site	None
Perf improvement	1 site	None

The only scenario where a real application could be affected is if it connects through a non-conforming TDS intermediary (proxy/load balancer) that rewrites token lengths to values exceeding these bounds. Standard SQL Server, Azure SQL, and conforming TDS 7.x/8.0 implementations will never produce values exceeding any of these limits.

Test infrastructure

• Added OnSQLBatchCompleted hook to GenericTdsServer for post-login TDS token injection
• Added OnAuthenticationResponseCompleted hook usage for login-phase token injection
• Per-test instance fixtures (IDisposable) for proper test isolation — each test gets a fresh TdsServerFixture
• 14 unit tests covering all bounds checks (login-phase, post-login, positive/negative boundary conditions)

Checklist

• Tests added or updated
• No breaking changes introduced
• PLP regression verified (test 9 confirms valid PLP sentinel is not rejected)
• 100% patch coverage on new bounds checks

[Copilot]

Pull request overview

This PR hardens the TDS parser against pre-auth (and post-auth) denial-of-service vectors by adding upper bounds checks on token/data lengths read from the wire, preventing unbounded allocations when a server spoofs lengths.

Changes:

• Added parser-side bounds checks for multiple TDS tokens/fields (SessionState, FedAuthInfo, FeatureExtAck, EnvChange PromoteTransaction, DateTime payloads, ReturnValue non‑PLP, Vector).
• Introduced new protocol max constants in TdsEnums and expanded simulated-server infrastructure to inject malicious tokens in both login and batch responses.
• Added new simulated-server unit tests covering the new rejection paths and a PLP regression guard.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/TdsTokenBoundsTests.cs	New simulated-server tests covering several oversized-length scenarios (login + post-login injection).
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/FeatureExtAckBoundsTests.cs	New tests for FeatureExtAck oversized length handling and boundary acceptance.
src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/GenericTdsServer.cs	Adds a post-login SQL batch hook to allow response token injection in tests.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs	Implements the core bounds checks in the TDS parser and fixes PLP/non‑PLP handling in ReturnValue.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs	Adds new max-length constants used by the parser checks.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs	Adds bounds validation for SRECOVERY inner state option parsing in FeatureExtAck handling.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.84%. Comparing base (5ac26c9) to head (1918e49).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4340      +/-   ##
==========================================
- Coverage   66.50%   64.84%   -1.67%     
==========================================
  Files         285      280       -5     
  Lines       43311    66191   +22880     
==========================================
+ Hits        28806    42923   +14117     
- Misses      14505    23268    +8763     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	64.84% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[paulmedynski]

Self-review: Test coverage annotations

Each bounds check added to driver code has been annotated with the specific test(s) that exercise it. Summary:

Driver change	Test(s)
TdsEnums.cs — MaxTokenDataLength, MaxPromoteTransactionLength, MaxDateTimeLength	All 12 tests validate these thresholds
TdsParser.cs — PromoteTransaction bounds check (L3351)	EnvChange_PromoteTransaction_OversizedLength_ThrowsParsingError, BatchResponse_PromoteTransaction_OversizedLength_ThrowsParsingError
TdsParser.cs — FeatureExtAck bounds check (L3853)	FeatureExtAck_OversizedDataLength_ThrowsParsingError, FeatureExtAck_MaxAllowedDataLength_DoesNotThrow
TdsParser.cs — SessionState total length (L4178)	SessionState_OversizedTotalLength_ThrowsParsingError
TdsParser.cs — SessionState inner option (L4234)	SessionState_OversizedInnerOptionLength_ThrowsParsingError
TdsParser.cs — FedAuthInfo token length (L4455)	FedAuthInfo_OversizedTokenLength_ThrowsParsingError
TdsParser.cs — ReturnValue non-PLP length (L4936)	BatchResponse_ReturnValue_OversizedLength_ThrowsParsingError, BatchResponse_ReturnValue_PlpUnknownLength_Succeeds
TdsParser.cs — DateTime length (L7143)	BatchResponse_DateTime_OversizedLength_ThrowsParsingError
TdsParser.cs — Binary/Vector sql_variant (L7405)	BatchResponse_SqlVariantBinary_OversizedLength_ThrowsParsingError
SqlConnectionInternal.cs — SRECOVERY inner state (L1349)	SRecovery_MalformedInnerStateLength_ThrowsParsingError

All 12 tests pass in ~19s.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.