Security improvements#1102
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses reported security issues in YamlDotNet by preventing unbounded memory growth from (1) merge expansion in MergingParser and (2) unbounded string interning for anchors/tags/keys. It also updates a few projects’ target frameworks (including avoiding .NET Framework builds on Linux).
Changes:
- Stop interning YAML-derived anchor/tag/key strings by switching from
string.Interntostring.IsInterned(...) ?? value. - Add a configurable maximum parsing-event limit to
MergingParserand tests to cover merge-key “bomb” scenarios. - Adjust several project TFMs (net8 → net10 in samples; and conditional TFMs to avoid net47 on Linux).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| YamlDotNet/Core/TagName.cs | Avoids interning tag names derived from input. |
| YamlDotNet/Core/AnchorName.cs | Avoids interning anchor names derived from input. |
| YamlDotNet/Core/Events/Scalar.cs | Avoids interning scalar keys derived from input. |
| YamlDotNet/Core/MergingParser.cs | Adds an event-count limit to mitigate merge expansion memory exhaustion. |
| YamlDotNet.Test/YamlDotNet.Test.csproj | Makes TFMs OS-conditional to avoid running .NET Framework on Linux. |
| YamlDotNet.Test/Serialization/MergingParserTests.cs | Adds tests for event-limit enforcement in merge scenarios. |
| YamlDotNet.Test/Core/StringInterningTests.cs | Adds tests asserting input strings are not force-interned. |
| YamlDotNet.Samples/YamlDotNet.Samples.csproj | Bumps samples to net10.0. |
| YamlDotNet.Samples.Fsharp/YamlDotNet.Samples.Fsharp.fsproj | Makes TFMs OS-conditional and adds net10.0. |
| YamlDotNet.Fsharp.Test/YamlDotNet.Fsharp.Test.fsproj | Makes TFMs OS-conditional and adds net10.0. |
| YamlDotNet.Core7AoTCompileTest/YamlDotNet.Core7AoTCompileTest.csproj | Bumps AoT compile test to net10.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@EdwardCooke, thanks! Looking for official release. It will be happy to disclose both issues under https://github.com/aaubry/YamlDotNet/security. It will be easily discoverable by all NuGet users. FYI: @danfiedler-msft, as you reported first issue. |
A couple of vulnerabilities were received.
Piotr Kiełkowicz - Cisco
Also bumps net8 to net10 on a couple of projects and makes it so net47 isn't ran on Linux environments.