Add TDS token data length bounds checks

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs

@@ -400,7 +400,7 @@ internal SqlConnectionInternal(
 
             try
             {
-                // If we want to consider pool operations against the overall connect timeout, 
+                // If we want to consider pool operations against the overall connect timeout,
                 // use the provided timeout. Otherwise, start a fresh timeout to receive the full
                 // connect timeout.
                 _timeout = ResolveLoginTimeout(timeout, connectionOptions.ConnectTimeout);
@@ -1346,6 +1346,11 @@ internal void OnFeatureExtAck(int featureId, byte[] data)
                             len = bLen;
                         }
 
+                        if (len < 0 || len > data.Length - i)
+                        {
+                            throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, len);
+                        }
+
                         byte[] stateData = new byte[len];
                         Buffer.BlockCopy(data, i, stateData, 0, len);
                         i += len;

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlUtil.cs

@@ -731,6 +731,10 @@ internal static Exception ParsingErrorLength(ParsingErrorState state, int length
         {
             return ADP.InvalidOperation(StringsHelper.GetString(Strings.SQL_ParsingErrorLength, ((int)state).ToString(CultureInfo.InvariantCulture), length));
         }
+        internal static Exception ParsingErrorLength(ParsingErrorState state, uint length)
+        {
+            return ADP.InvalidOperation(StringsHelper.GetString(Strings.SQL_ParsingErrorLength, ((int)state).ToString(CultureInfo.InvariantCulture), length));
+        }
         internal static Exception ParsingErrorStatus(ParsingErrorState state, int status)
         {
             return ADP.InvalidOperation(StringsHelper.GetString(Strings.SQL_ParsingErrorStatus, ((int)state).ToString(CultureInfo.InvariantCulture), status));

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs

@@ -84,6 +84,17 @@ internal static class TdsEnums
         public const int MAX_PACKET_SIZE = 32768;
         public const int MAX_SERVER_USER_NAME = 256;  // obtained from luxor
 
+        // Maximum allowed data length for token payloads (feature ext ack,
+        // session state, fedauth info). Prevents a malicious server from causing
+        // unbounded memory allocation via spoofed token length fields.
+        internal const int MaxTokenDataLength = 1 << 20; // 1 MB
+
+        // Maximum allowed data length for a DTC promote transaction propagation token.
+        internal const int MaxPromoteTransactionLength = 1 << 16; // 64 KB
+
+        // Maximum valid wire size for datetime types (DateTimeOffset = 5 time + 3 date + 2 offset).
+        internal const int MaxDateTimeLength = 10;
+
         // Severity  0 - 10 indicates informational (non-error) messages
         // Severity 11 - 16 indicates errors that can be corrected by user (syntax errors, etc...)
         // Severity 17 - 19 indicates failure due to insufficient resources in the server

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs

@@ -3348,6 +3348,10 @@ private TdsOperationStatus TryProcessEnvChange(int tokenLength, TdsParserStateOb
                             // new value has 4 byte length
                             return result;
                         }
+                        if (env._newLength < 0 || env._newLength > TdsEnums.MaxPromoteTransactionLength)
+                        {
+                            throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, env._newLength);
+                        }
                         // read new value with 4 byte length
                         env._newBinValue = new byte[env._newLength];
                         result = stateObj.TryReadByteArray(env._newBinValue, env._newLength);
@@ -3846,10 +3850,15 @@ private TdsOperationStatus TryProcessFeatureExtAck(TdsParserStateObject stateObj
                     {
                         return result;
                     }
-                    byte[] data = new byte[dataLen];
-                    if (dataLen > 0)
+                    if (dataLen > (uint)TdsEnums.MaxTokenDataLength)
+                    {
+                        throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, dataLen);
+                    }
+                    int dataLength = (int)dataLen;
+                    byte[] data = new byte[dataLength];
+                    if (dataLength > 0)
                     {
-                        result = stateObj.TryReadByteArray(data, checked((int)dataLen));
+                        result = stateObj.TryReadByteArray(data, dataLength);
                         if (result != TdsOperationStatus.Done)
                         {
                             return result;
@@ -4169,6 +4178,10 @@ private TdsOperationStatus TryProcessSessionState(TdsParserStateObject stateObj,
             {
                 throw SQL.ParsingErrorLength(ParsingErrorState.SessionStateLengthTooShort, length);
             }
+            if (length > TdsEnums.MaxTokenDataLength)
+            {
+                throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, length);
+            }
             uint seqNum;
             TdsOperationStatus result = stateObj.TryReadUInt32(out seqNum);
             if (result != TdsOperationStatus.Done)
@@ -4218,6 +4231,10 @@ private TdsOperationStatus TryProcessSessionState(TdsParserStateObject stateObj,
                         return result;
                     }
                 }
+                if (stateLen < 0 || stateLen > TdsEnums.MaxTokenDataLength)
+                {
+                    throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, stateLen);
+                }
                 byte[] buffer = null;
                 lock (sdata._delta)
                 {
@@ -4435,6 +4452,10 @@ private TdsOperationStatus TryProcessFedAuthInfo(TdsParserStateObject stateObj,
                 SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.TryProcessFedAuthInfo|ERR> FEDAUTHINFO token stream length too short for CountOfInfoIDs.");
                 throw SQL.ParsingErrorLength(ParsingErrorState.FedAuthInfoLengthTooShortForCountOfInfoIds, tokenLen);
             }
+            if (tokenLen > TdsEnums.MaxTokenDataLength)
+            {
+                throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, tokenLen);
+            }
 
             // read how many FedAuthInfo options there are
             uint optionsCount;
@@ -4912,14 +4933,20 @@ internal TdsOperationStatus TryProcessReturnValue(int length,
             }
 
             // always read as sql types
-            Debug.Assert(valLen < (ulong)(int.MaxValue), "ProcessReturnValue received data size > 2Gb");
-
-            int intlen = valLen > (ulong)(int.MaxValue) ? int.MaxValue : (int)valLen;
+            int intlen;
 
             if (rec.metaType.IsPlp)
             {
                 intlen = int.MaxValue;    // If plp data, read it all
             }
+            else if (valLen > (ulong)int.MaxValue)
+            {
+                throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, unchecked((int)valLen));
+            }
+            else
+            {
+                intlen = (int)valLen;
+            }
 
             if (rec.type == SqlDbTypeExtensions.Vector)
             {
@@ -7111,9 +7138,20 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value,
             return TdsOperationStatus.Done;
         }
 
+        // length originates as a single byte on the wire (nullable datetime length prefix),
+        // but is kept as int to match the TDS parsing API surface where all lengths are int.
+        // Using byte here would require casts at all call sites and silently truncate values
+        // from the sql_variant path where lenData is computed arithmetically.
         private TdsOperationStatus TryReadSqlDateTime(SqlBuffer value, byte tdsType, int length, byte scale, TdsParserStateObject stateObj)
         {
-            Span<byte> datetimeBuffer = ((uint)length <= 16) ? stackalloc byte[16] : new byte[length];
+            // DateTimeOffset is the largest datetime type at 10 bytes (5 time + 3 date + 2 offset).
+            // Reject anything larger to prevent heap allocation from spoofed metadata.
+            if (length < 0 || length > TdsEnums.MaxDateTimeLength)
+            {
+                throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, length);
+            }
+
+            Span<byte> datetimeBuffer = stackalloc byte[TdsEnums.MaxDateTimeLength];
 
             TdsOperationStatus result = stateObj.TryReadByteArray(datetimeBuffer, length);
             if (result != TdsOperationStatus.Done)
@@ -7369,7 +7407,10 @@ internal TdsOperationStatus TryReadSqlValueInternal(SqlBuffer value, byte tdsTyp
                 case TdsEnums.SQLVECTOR:
                     {
                         // Note: Better not come here with plp data!!
-                        Debug.Assert(length <= TdsEnums.MAXSIZE);
+                        if (length < 0 || length > TdsEnums.MAXSIZE)
+                        {
+                            throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, length);
+                        }
                         result = stateObj.TryReadByteArrayWithContinue(length, isPlp: false, out byte[] b);
                         if (result != TdsOperationStatus.Done)
                         {

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionEnhancedRoutingTests.cs

@@ -14,6 +14,8 @@ namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests;
 /// <summary>
 /// Tests connection routing using the enhanced routing feature extension and envchange token
 /// </summary>
+// TODO: Do we need this collection?  It serializes all tests within it, which we probably don't
+// need since each test uses its own TDS Server with ephemeral listen port.
 [Collection("SimulatedServerTests")]
 public class ConnectionEnhancedRoutingTests
 {

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs

@@ -12,6 +12,8 @@
 
 namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests
 {
+    // TODO: Do we need this collection?  It serializes all tests within it, which we probably don't
+    // need since each test uses its own TDS Server with ephemeral listen port.
     [Collection("SimulatedServerTests")]
     public class ConnectionFailoverTests
     {

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionReadOnlyRoutingTests.cs

@@ -11,6 +11,8 @@
 
 namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests
 {
+    // TODO: Do we need this collection?  It serializes all tests within it, which we probably don't
+    // need since each test uses its own TDS Server with ephemeral listen port.
     [Collection("SimulatedServerTests")]
     public class ConnectionReadOnlyRoutingTests
     {
@@ -71,7 +73,7 @@ public void RecursivelyRoutedConnection(int layers)
                     router.Start();
                     routingLayers.Push(router);
                     lastEndpoint = router.EndPoint;
-                    lastConnectionString = (new SqlConnectionStringBuilder() { 
+                    lastConnectionString = (new SqlConnectionStringBuilder() {
                         DataSource = $"localhost,{lastEndpoint.Port}",
                         ApplicationIntent = ApplicationIntent.ReadOnly,
                         Encrypt = false
@@ -114,8 +116,8 @@ public async Task RecursivelyRoutedAsyncConnection(int layers)
                     router.Start();
                     routingLayers.Push(router);
                     lastEndpoint = router.EndPoint;
-                    lastConnectionString = (new SqlConnectionStringBuilder() { 
-                        DataSource = $"localhost,{lastEndpoint.Port}", 
+                    lastConnectionString = (new SqlConnectionStringBuilder() {
+                        DataSource = $"localhost,{lastEndpoint.Port}",
                         ApplicationIntent = ApplicationIntent.ReadOnly,
                         Encrypt = false
                     }).ConnectionString;

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionRoutingTests.cs

@@ -10,6 +10,8 @@
 
 namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests
 {
+    // TODO: Do we need this collection?  It serializes all tests within it, which we probably don't
+    // need since each test uses its own TDS Server with ephemeral listen port.
     [Collection("SimulatedServerTests")]
     public class ConnectionRoutingTests
     {
@@ -194,7 +196,7 @@ public void NetworkTimeoutAtRoutedLocation_RetryDisabled_ShouldFail()
             // Act
             var e = Assert.Throws<SqlException>(connection.Open);
 
-            // Assert 
+            // Assert
             Assert.Equal(ConnectionState.Closed, connection.State);
             Assert.Contains("Connection Timeout Expired", e.Message);
         }

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionRoutingTestsAzure.cs

@@ -10,6 +10,8 @@
 
 namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests
 {
+    // TODO: Do we need this collection?  It serializes all tests within it, which we probably don't
+    // need since each test uses its own TDS Server with ephemeral listen port.
     [Collection("SimulatedServerTests")]
     public class ConnectionRoutingTestsAzure : IDisposable
     {
@@ -21,8 +23,8 @@ public ConnectionRoutingTestsAzure()
             adpHelper.AddAzureSqlServerEndpoint("localhost");
         }
 
-        public void Dispose() 
-        { 
+        public void Dispose()
+        {
             adpHelper.Dispose();
         }
 
@@ -192,7 +194,7 @@ public void NetworkTimeoutAtRoutedLocation_RetryDisabled_ShouldFail()
             // Act
             var e = Assert.Throws<SqlException>(connection.Open);
 
-            // Assert 
+            // Assert
             Assert.Equal(ConnectionState.Closed, connection.State);
             Assert.Contains("Connection Timeout Expired", e.Message);
         }