mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 22:55:08 +08:00
Marc Schier
1c312e5559
# Description Two coordinated fixes that close issue #3546 and remove the CI gap that allowed the regression to slip past `dotnet test`: 1. **`BinaryDecoder` hardens matrix-dimension handling** `Stack/Opc.Ua.Types/Encoders/BinaryDecoder.cs` — wraps the matrix-construction switch in `ReadVariantValue` (all 24 BuiltInType cases) and both `ReadEncodeableMatrix<T>` overloads in a try/catch that converts the deliberate `ArgumentException` thrown by `MatrixOf<T>(values, dimensions)` validation (overflow / negative / length-mismatch) into a `ServiceResultException(BadDecodingError)`. Attacker-controlled wire dimensions now flow through the standard decoder rejection channel instead of escaping as an unhandled exception. The `MatrixOf<T>` ctor validation itself (security hardening at `MatrixOf.cs:178-217`) is unchanged. 2. **`FuzzCrashAssets` is now assertive** `Fuzzing/Common/Fuzz.Tests/FuzzTargetTestsBase.cs` — replaces silent log-and-count with `Assert.That(failures, Is.Empty, …listing failed assets)`. CI now breaks loud the moment a residual `Assets/crash-*` reproduces on any target. `.github/agents/fuzz-tester.agent.md` doc note rewritten to match. The originally reported "DataTypeNode / VariableNode `IsEqual` non-determinism" symptom turned out to already be implicitly resolved by the modern `Equals` overhaul (`ArrayOf<T>` value-equality, `ExtensionObject.Equals` deep-compare, `IEquatable<T>` on `ReferenceNode` / `RolePermissionType`). The 8 new synthetic round-trip tests + 6 permanent corpus seeds confirm this end-to-end. ## Regression coverage * `Tests/Opc.Ua.Types.Tests/Encoders/BinaryDecoderTests.cs` — 3 new tests lock in the `BadDecodingError` contract for malformed matrix dimensions (overflow, negative, length-mismatch). * `Fuzzing/Opc.Ua.Encoders.Fuzz.Tests/Issue3546RoundTripTests.cs` — 8 synthetic round-trip tests for populated `DataTypeNode` and `VariableNode`, exercising the implicated fields (non-empty References, RolePermissions, `DataTypeDefinition` ExtensionObject body, `Value` Variant carrying ExtensionObject, `ArrayDimensions` empty / null / non-empty). * `Fuzzing/Opc.Ua.Encoders.Fuzz.Tools/Encoders.Testcases.cs` + `Fuzzing/Opc.Ua.Encoders.Fuzz.Corpus/Testcases.{Binary,Json,Xml}/` — `DataTypeNodeMessage` / `VariableNodeMessage` seed generators plus 6 generated corpus files. These feed `FuzzGoodTestcases` automatically on every test run. ## Verification | Suite | TFM | Result | |---|---|---| | `Opc.Ua.Encoders.Fuzz.Tests` | net10.0 | **4625** / 0 | | `Opc.Ua.Encoders.Fuzz.Tests` | net48 | **4625** / 0 | | `Opc.Ua.Types.Tests` (BinaryDecoder / Matrix / Variant / EncodeableMatrixRoundTrip subset) | net10.0 | **985** / 0 | The single pre-existing `Assets/crash-0483107a22caf892cff4f795c96ab14c581a8f55` no longer escapes any fuzz target — the matrix-dimensions fix turns it into a clean parser-rejected input. ## Related Issues - Fixes #3546 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran the affected tests locally (`Opc.Ua.Encoders.Fuzz.Tests` on net10.0 + net48, targeted `Opc.Ua.Types.Tests` subset on net10.0); full UA.slnx still pending CI. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received.