Validate sizes and offsets when parsing NTLM challenge (#128692)

The managed NTLM client previously trusted the wire fields in
`CHALLENGE_MESSAGE` without checking that the declared offsets, lengths,
and AV pair sizes fit within the supplied blob. A malformed or truncated
challenge could cause out-of-range slice exceptions or silently misparse
target info. This addresses the long-standing `// TODO: Validate size
and offsets` in `ProcessChallenge`.

Changes:

- `ProcessChallenge` rejects challenges smaller than the fixed
`ChallengeMessage` header before reading.
- `GetField` guards against negative offsets and uses `offset >
payload.Length - length` so a huge offset can no longer wrap around to
look in-range.
- `ProcessTargetInfo` validates that each AV pair''s declared body fits
in the remaining bytes and that the `Timestamp` AV is at least 8 bytes.
On malformed input it returns `null`, and `ProcessChallenge` surfaces
that as `InvalidToken` to the caller.
- `targetInfoBuffer` is sized to include the trailing EOL bytes that the
method writes.

Behavior on well-formed challenges is unchanged; only malformed inputs
now fail cleanly with `InvalidToken` instead of throwing or producing
undefined data.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: rzikm

src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.ManagedNtlm.cs

@@ -378,7 +378,7 @@ private static ReadOnlySpan<byte> GetField(MessageField field, ReadOnlySpan<byte
                 int offset = field.PayloadOffset;
                 int length = field.Length;
 
-                if (length == 0 || offset + length > payload.Length)
+                if (length == 0 || offset < 0 || offset > payload.Length - length)
                 {
                     return ReadOnlySpan<byte>.Empty;
                 }
@@ -508,7 +508,7 @@ private unsafe void WriteChannelBindingHash(Span<byte> hashBuffer)
                 }
             }
 
-            private byte[] ProcessTargetInfo(ReadOnlySpan<byte> targetInfo, out DateTime time, out bool hasNbNames)
+            private byte[]? ProcessTargetInfo(ReadOnlySpan<byte> targetInfo, out DateTime time, out bool hasNbNames)
             {
                 int spnSize = _spn != null ? Encoding.Unicode.GetByteCount(_spn) : 0;
 
@@ -518,10 +518,11 @@ private byte[] ProcessTargetInfo(ReadOnlySpan<byte> targetInfo, out DateTime tim
                 }
 
                 bool hasNbComputerName = false, hasNbDomainName = false;
-                byte[] targetInfoBuffer = new byte[targetInfo.Length + 20 /* channel binding */ + 4 + spnSize /* SPN */ + 8 /* flags */];
+                byte[] targetInfoBuffer = new byte[targetInfo.Length + 20 /* channel binding */ + 4 + spnSize /* SPN */ + 8 /* flags */ + 4 /* EOL */];
                 int targetInfoOffset = 0;
 
                 time = DateTime.UtcNow;
+                hasNbNames = false;
 
                 if (targetInfo.Length > 0)
                 {
@@ -538,8 +539,19 @@ private byte[] ProcessTargetInfo(ReadOnlySpan<byte> targetInfo, out DateTime tim
                             break;
                         }
 
+                        // Make sure the AV pair fits in the remaining target info bytes.
+                        if (length > info.Length - 4)
+                        {
+                            return null;
+                        }
+
                         if (ID == AvId.Timestamp)
                         {
+                            if (length < 8)
+                            {
+                                return null;
+                            }
+
                             time = DateTime.FromFileTimeUtc(BinaryPrimitives.ReadInt64LittleEndian(info.Slice(4, 8)));
                         }
                         else if (ID == AvId.TargetName || ID == AvId.ChannelBindings)
@@ -616,7 +628,12 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS
             // This gets decoded byte blob and returns response in binary form.
             private unsafe byte[]? ProcessChallenge(ReadOnlySpan<byte> blob, out NegotiateAuthenticationStatusCode statusCode)
             {
-                // TODO: Validate size and offsets
+                // The challenge must be at least large enough to hold the fixed-size header.
+                if (blob.Length < sizeof(ChallengeMessage))
+                {
+                    statusCode = NegotiateAuthenticationStatusCode.InvalidToken;
+                    return null;
+                }
 
                 ref readonly ChallengeMessage challengeMessage = ref MemoryMarshal.AsRef<ChallengeMessage>(blob.Slice(0, sizeof(ChallengeMessage)));
 
@@ -649,7 +666,12 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS
                 }
 
                 ReadOnlySpan<byte> targetInfo = GetField(challengeMessage.TargetInfo, blob);
-                byte[] targetInfoBuffer = ProcessTargetInfo(targetInfo, out DateTime time, out bool hasNbNames);
+                byte[]? targetInfoBuffer = ProcessTargetInfo(targetInfo, out DateTime time, out bool hasNbNames);
+                if (targetInfoBuffer is null)
+                {
+                    statusCode = NegotiateAuthenticationStatusCode.InvalidToken;
+                    return null;
+                }
 
                 // If NTLM v2 authentication is used and the CHALLENGE_MESSAGE does not contain both
                 // MsvAvNbComputerName and MsvAvNbDomainName AVPairs and either Integrity is TRUE or

src/libraries/System.Net.Security/tests/UnitTests/NegotiateAuthenticationTests.cs

@@ -435,5 +435,99 @@ public void NegotiateCorrectExchangeTest(bool requestMIC, bool requestConfidenti
             }
             while (!ntAuth.IsAuthenticated);
         }
+
+        public static IEnumerable<object[]> MalformedChallengeBlobs()
+        {
+            // Truncated below sizeof(ChallengeMessage). The fixed header is 56 bytes.
+            yield return new object[] { "TooShort", (Func<byte[], byte[]>)(blob => blob.AsSpan(0, 40).ToArray()) };
+
+            // TargetInfo payload offset points past the end of the blob.
+            yield return new object[] { "TargetInfoOffsetOutOfRange", (Func<byte[], byte[]>)(blob =>
+            {
+                byte[] copy = (byte[])blob.Clone();
+                BinaryPrimitives.WriteUInt32LittleEndian(copy.AsSpan(44), (uint)(copy.Length + 100));
+                return copy;
+            }) };
+
+            // TargetInfo length extends past the end of the blob.
+            yield return new object[] { "TargetInfoLengthOutOfRange", (Func<byte[], byte[]>)(blob =>
+            {
+                byte[] copy = (byte[])blob.Clone();
+                BinaryPrimitives.WriteUInt16LittleEndian(copy.AsSpan(40), ushort.MaxValue);
+                BinaryPrimitives.WriteUInt16LittleEndian(copy.AsSpan(42), ushort.MaxValue);
+                return copy;
+            }) };
+
+            // TargetInfo payload offset is negative (would underflow without the offset < 0 guard).
+            yield return new object[] { "TargetInfoOffsetNegative", (Func<byte[], byte[]>)(blob =>
+            {
+                byte[] copy = (byte[])blob.Clone();
+                BinaryPrimitives.WriteInt32LittleEndian(copy.AsSpan(44), -8);
+                return copy;
+            }) };
+
+            // AV pair declares a length longer than the remaining TargetInfo bytes.
+            yield return new object[] { "AvPairOverrun", (Func<byte[], byte[]>)(blob =>
+            {
+                byte[] copy = (byte[])blob.Clone();
+                int targetInfoOffset = (int)BinaryPrimitives.ReadUInt32LittleEndian(copy.AsSpan(44));
+                // Overwrite the first AV pair length so it extends past the end of TargetInfo.
+                BinaryPrimitives.WriteUInt16LittleEndian(copy.AsSpan(targetInfoOffset + 2), ushort.MaxValue);
+                return copy;
+            }) };
+
+            // Timestamp AV pair body is shorter than the required 8 bytes.
+            yield return new object[] { "ShortTimestamp", (Func<byte[], byte[]>)(blob =>
+            {
+                byte[] copy = (byte[])blob.Clone();
+                int targetInfoOffset = (int)BinaryPrimitives.ReadUInt32LittleEndian(copy.AsSpan(44));
+                int targetInfoLength = BinaryPrimitives.ReadUInt16LittleEndian(copy.AsSpan(40));
+                Span<byte> ti = copy.AsSpan(targetInfoOffset, targetInfoLength);
+                // Walk to the Timestamp AV (AvId 7) emitted by FakeNtlmServer and shorten it.
+                int pos = 0;
+                while (pos + 4 <= ti.Length)
+                {
+                    ushort id = BinaryPrimitives.ReadUInt16LittleEndian(ti.Slice(pos));
+                    ushort len = BinaryPrimitives.ReadUInt16LittleEndian(ti.Slice(pos + 2));
+                    if (id == 7 /* Timestamp */)
+                    {
+                        BinaryPrimitives.WriteUInt16LittleEndian(ti.Slice(pos + 2), 4);
+                        break;
+                    }
+                    pos += 4 + len;
+                }
+                return copy;
+            }) };
+        }
+
+        [ConditionalTheory(typeof(NegotiateAuthenticationTests), nameof(UseManagedNtlm))]
+        [MemberData(nameof(MalformedChallengeBlobs))]
+        public void NtlmMalformedChallenge_ReturnsInvalidToken(string scenario, Func<byte[], byte[]> corruptor)
+        {
+            _ = scenario;
+
+            using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
+            NegotiateAuthentication ntAuth = new NegotiateAuthentication(
+                new NegotiateAuthenticationClientOptions
+                {
+                    Package = "NTLM",
+                    Credential = s_testCredentialRight,
+                    TargetName = "HTTP/foo",
+                });
+
+            NegotiateAuthenticationStatusCode statusCode;
+            byte[]? negotiateBlob = ntAuth.GetOutgoingBlob((byte[])null, out statusCode);
+            Assert.Equal(NegotiateAuthenticationStatusCode.ContinueNeeded, statusCode);
+            Assert.NotNull(negotiateBlob);
+
+            byte[]? challengeBlob = fakeNtlmServer.GetOutgoingBlob(negotiateBlob);
+            Assert.NotNull(challengeBlob);
+
+            byte[] malformed = corruptor(challengeBlob);
+
+            byte[]? response = ntAuth.GetOutgoingBlob(malformed, out statusCode);
+            Assert.Equal(NegotiateAuthenticationStatusCode.InvalidToken, statusCode);
+            Assert.Null(response);
+        }
     }
 }