Reject malformed and oversized typed MCP arguments

Enforce the 4 MiB limit using UTF-8 bytes before parsing, and reject non-null Value envelopes without an explicit UaType instead of dispatching Variant.Null. Add exact multibyte boundary and both-call-tool transport regressions while preserving compact nulls and typed defaults.
2026-10-28 22:55:49 +08:00 · 2026-10-15 20:10:01 +02:00
parent bd571a80bd
commit f62aecdb43
4 changed files with 141 additions and 1 deletions
@@ -35,6 +35,7 @@ using System.Threading.Tasks;
 using NUnit.Framework;
 using Opc.Ua;
 using Opc.Ua.Mcp.Tools;
+using Opc.Ua.Tools.Tests.McpAuthorization;

 namespace Opc.Ua.Tools.Tests.Mcp
 {
@@ -43,6 +44,36 @@ namespace Opc.Ua.Tools.Tests.Mcp
    {
        private const string kUnknownSessionName = "definitely-not-connected";

+        /// <summary>
+        /// Both public call tools reject malformed variant envelopes before any method reaches the transport.
+        /// </summary>
+        [TestCase(false, "[{\"Value\":42}]")]
+        [TestCase(true, "[{\"Value\":42}]")]
+        [TestCase(false, "[{\"UaTyp\":6,\"Value\":42}]")]
+        [TestCase(true, "[{\"UaTyp\":6,\"Value\":42}]")]
+        [TestCase(false, "[{\"uaType\":6,\"Value\":42}]")]
+        [TestCase(true, "[{\"uaType\":6,\"Value\":42}]")]
+        [TestCase(false, "[{\"UaType\":null,\"Value\":42}]")]
+        [TestCase(true, "[{\"UaType\":null,\"Value\":42}]")]
+        public async Task CallToolsRejectMissingTypesBeforeDispatchAsync(bool convenience, string arguments)
+        {
+            await using McpGenericGenerationTestContext fixture =
+                await McpGenericGenerationTestContext.CreateAsync().ConfigureAwait(false);
+
+            string json = convenience
+                ? await ConvenienceTools.CallMethodAsync(
+                    fixture.Manager, "ns=1;i=100", "ns=1;i=101",
+                    inputArgumentsJson: arguments).ConfigureAwait(false)
+                : await MethodServiceTools.CallAsync(
+                    fixture.Manager, "ns=1;i=100", "ns=1;i=101",
+                    inputArgumentsJson: arguments).ConfigureAwait(false);
+
+            Assert.That(fixture.Mutations, Is.Empty);
+            using JsonDocument document = JsonDocument.Parse(json);
+            Assert.That(document.RootElement.GetProperty("error").GetBoolean(), Is.True);
+            Assert.That(document.RootElement.GetProperty("statusCode").GetString(), Is.EqualTo("BadDecodingError"));
+        }
+
        [Test]
        public async Task CallAsyncWithValidSessionReturnsStatusJsonAsync()
        {
@@ -31,6 +31,7 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
+using System.Linq;
 using System.Text;
 using System.Text.Json;
 using NUnit.Framework;
@@ -310,6 +311,101 @@ namespace Opc.Ua.Tools.Tests.Mcp
                legacy ? ["text"] : null, json, ServiceMessageContext.Create(null!)));
        }

+        /// <summary>
+        /// Multibyte arguments are bounded by their UTF-8 payload size, including the exact limit.
+        /// </summary>
+        [TestCase(-1)]
+        [TestCase(0)]
+        [TestCase(1)]
+        public void TypedCallArgumentsEnforceUtf8ByteLimit(int byteOffset)
+        {
+            const int maxBytes = 4 * 1024 * 1024;
+            string value = new('\u4E2D', 2700);
+            string item = "{\"UaType\":12,\"Value\":\"" + value + "\"}";
+            string prefix = "[" + string.Join(",", Enumerable.Repeat(item, 511)) + ",{\"UaType\":12,\"Value\":\"";
+            const string suffix = "\"}]";
+            int padding = maxBytes + byteOffset - Encoding.UTF8.GetByteCount(prefix) -
+                Encoding.UTF8.GetByteCount(value) - suffix.Length;
+            string finalValue = value + new string('x', padding);
+            string json = prefix + finalValue + suffix;
+            IServiceMessageContext context = ServiceMessageContext.Create(null!);
+
+            Assert.That(Encoding.UTF8.GetByteCount(json), Is.EqualTo(maxBytes + byteOffset));
+            Assert.That(json, Has.Length.LessThan(maxBytes));
+            if (byteOffset > 0)
+            {
+                ServiceResultException error = Assert.Throws<ServiceResultException>(() =>
+                    OpcUaJsonHelper.ParseCallArguments(null, json, context))!;
+                Assert.That(error.StatusCode, Is.EqualTo(StatusCodes.BadEncodingLimitsExceeded));
+            }
+            else
+            {
+                ArrayOf<Variant> decoded = OpcUaJsonHelper.ParseCallArguments(null, json, context);
+                Assert.That(decoded.Count, Is.EqualTo(512));
+                Assert.That(decoded[0].TryGetValue(out string first), Is.True);
+                Assert.That(first, Is.EqualTo(value));
+                Assert.That(decoded[^1].TryGetValue(out string last), Is.True);
+                Assert.That(last, Is.EqualTo(finalValue));
+            }
+        }
+
+        /// <summary>
+        /// Oversized multibyte input is refused before malformed JSON can reach the parser.
+        /// </summary>
+        [TestCase(false)]
+        [TestCase(true)]
+        public void TypedCallArgumentsRejectOversizeBeforeParsing(bool multibyte)
+        {
+            const int maxBytes = 4 * 1024 * 1024;
+            string json = multibyte ? new('\u4E2D', maxBytes / 3 + 1) : new('x', maxBytes + 1);
+
+            ServiceResultException error = Assert.Throws<ServiceResultException>(() =>
+                OpcUaJsonHelper.ParseCallArguments(null, json, ServiceMessageContext.Create(null!)))!;
+
+            Assert.That(error.StatusCode, Is.EqualTo(StatusCodes.BadEncodingLimitsExceeded));
+        }
+
+        /// <summary>
+        /// A non-null value without a usable type tag must not silently become Variant.Null.
+        /// </summary>
+        [TestCase("[{\"Value\":42}]")]
+        [TestCase("[{\"Value\":0}]")]
+        [TestCase("[{\"Value\":false}]")]
+        [TestCase("[{\"Value\":\"\"}]")]
+        [TestCase("[{\"Value\":[]}]")]
+        [TestCase("[{\"Value\":{}}]")]
+        [TestCase("[{\"UaTyp\":6,\"Value\":42}]")]
+        [TestCase("[{\"uaType\":6,\"Value\":42}]")]
+        [TestCase("[{\"UaType\":null,\"Value\":42}]")]
+        public void TypedCallArgumentsRejectMissingType(string json)
+        {
+            ServiceResultException error = Assert.Throws<ServiceResultException>(() =>
+                OpcUaJsonHelper.ParseCallArguments(null, json, ServiceMessageContext.Create(null!)))!;
+
+            Assert.That(error.StatusCode, Is.EqualTo(StatusCodes.BadDecodingError));
+        }
+
+        /// <summary>
+        /// Compact null encodings and omitted typed defaults remain valid alongside explicit zero and false.
+        /// </summary>
+        [Test]
+        public void TypedCallArgumentsPreserveNullAndTypedDefaults()
+        {
+            const string json =
+                "[null,{},{\"Value\":null},{\"UaType\":0},{\"UaType\":6}," +
+                "{\"UaType\":1,\"Value\":false},{\"UaType\":6,\"Value\":0}]";
+            ArrayOf<Variant> expected =
+            [
+                Variant.Null, Variant.Null, Variant.Null, Variant.Null,
+                Variant.From(0), Variant.From(false), Variant.From(0)
+            ];
+
+            ArrayOf<Variant> actual = OpcUaJsonHelper.ParseCallArguments(
+                null, json, ServiceMessageContext.Create(null!));
+
+            Assert.That(actual, Is.EqualTo(expected));
+        }
+
        [Test]
        public void VariantToObjectReturnsBooleanAsIs()
        {
@@ -95,6 +95,9 @@ legacy string array. It is an OPC UA JSON Variant array, for example
 Structure values use `UaType: 22` and a `Value` object containing `UaTypeId` and
 the structure's fields. Load custom types with `schema_load_types` first.
 Conflicting argument forms and invalid JSON are rejected before dispatch.
+Typed input is limited to 4 MiB of UTF-8 JSON and 1,024 arguments. A non-null
+`Value` requires an explicit `UaType`; a missing or misspelled type tag is not
+silently interpreted as a null argument. Null encodings and typed defaults remain supported.
 Existing `inputArguments` strings are never implicitly reinterpreted as JSON.

 Generic read/call results retain an ExtensionObject's `typeId` and decoded `body`
@@ -370,7 +370,9 @@ namespace Opc.Ua.Mcp.Serialization
                throw new ServiceResultException(StatusCodes.BadInvalidArgument,
                    "Specify either inputArguments or inputArgumentsJson, not both.");
            }
-            if (inputArgumentsJson.Length > 4 * 1024 * 1024)
+            const int maxInputBytes = 4 * 1024 * 1024;
+            if (inputArgumentsJson.Length > maxInputBytes ||
+                Encoding.UTF8.GetByteCount(inputArgumentsJson) > maxInputBytes)
            {
                throw new ServiceResultException(StatusCodes.BadEncodingLimitsExceeded,
                    "Typed method arguments exceed the 4 MiB input bound.");
@@ -390,6 +392,14 @@ namespace Opc.Ua.Mcp.Serialization
                throw new ServiceResultException(StatusCodes.BadInvalidArgument,
                    "Typed method arguments must be a UA JSON Variant array of at most 1024 items.");
            }
+            foreach (JsonNode? argument in array)
+            {
+                if (argument is JsonObject envelope && envelope["Value"] != null && envelope["UaType"] == null)
+                {
+                    throw new ServiceResultException(StatusCodes.BadDecodingError,
+                        "A non-null typed method argument Value requires an explicit UaType.");
+                }
+            }
            using var decoder = new JsonDecoder(new JsonObject { ["Arguments"] = array }.ToJsonString(), context);
            return decoder.ReadVariantArray("Arguments");
        }