diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 000000000..89ecc0464 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,13 @@ +# Reporting + +Please submit reports of security vulnerabilities and, if possible, code to reproduce the vulnerability. + +That code will be the basis for the fix. + +Please send reports to and instead of directly opening an issue detailing the finding. + +We will reach out and let you know when it's appropriate to open that issue and post that information. + +Though, likely the reproduction will be in the unit test and details included in the PR around the vulnerability. + +Reports will promptly be investigated and responded to. diff --git a/YamlDotNet.Core7AoTCompileTest/YamlDotNet.Core7AoTCompileTest.csproj b/YamlDotNet.Core7AoTCompileTest/YamlDotNet.Core7AoTCompileTest.csproj index 8bb0418da..75d2570e6 100644 --- a/YamlDotNet.Core7AoTCompileTest/YamlDotNet.Core7AoTCompileTest.csproj +++ b/YamlDotNet.Core7AoTCompileTest/YamlDotNet.Core7AoTCompileTest.csproj @@ -2,7 +2,7 @@ Exe - net8.0 + net10.0 true true enable diff --git a/YamlDotNet.Fsharp.Test/YamlDotNet.Fsharp.Test.fsproj b/YamlDotNet.Fsharp.Test/YamlDotNet.Fsharp.Test.fsproj index 7fc790f9a..a79029ee3 100644 --- a/YamlDotNet.Fsharp.Test/YamlDotNet.Fsharp.Test.fsproj +++ b/YamlDotNet.Fsharp.Test/YamlDotNet.Fsharp.Test.fsproj @@ -1,6 +1,7 @@  - net8.0;net47 + net10.0;net8.0;net4.7 + net10.0;net8.0 false ..\YamlDotNet.snk true diff --git a/YamlDotNet.Samples.Fsharp/YamlDotNet.Samples.Fsharp.fsproj b/YamlDotNet.Samples.Fsharp/YamlDotNet.Samples.Fsharp.fsproj index 72f23d127..fc3958489 100644 --- a/YamlDotNet.Samples.Fsharp/YamlDotNet.Samples.Fsharp.fsproj +++ b/YamlDotNet.Samples.Fsharp/YamlDotNet.Samples.Fsharp.fsproj @@ -2,7 +2,7 @@ Exe - net8.0 + net10.0 false 5 diff --git a/YamlDotNet.Samples/YamlDotNet.Samples.csproj b/YamlDotNet.Samples/YamlDotNet.Samples.csproj index b99891b49..9a9511b3f 100644 --- a/YamlDotNet.Samples/YamlDotNet.Samples.csproj +++ b/YamlDotNet.Samples/YamlDotNet.Samples.csproj @@ -1,7 +1,7 @@ - net8.0 + net10.0 false diff --git a/YamlDotNet.Test/Core/StringInterningTests.cs b/YamlDotNet.Test/Core/StringInterningTests.cs new file mode 100644 index 000000000..a7013be4f --- /dev/null +++ b/YamlDotNet.Test/Core/StringInterningTests.cs @@ -0,0 +1,82 @@ +// This file is part of YamlDotNet - A .NET library for YAML. +// Copyright (c) Antoine Aubry and contributors +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of +// this software and associated documentation files (the "Software"), to deal in +// the Software without restriction, including without limitation the rights to +// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies +// of the Software, and to permit persons to whom the Software is furnished to do +// so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +using System; +using Xunit; +using YamlDotNet.Core; +using YamlDotNet.Core.Events; + +namespace YamlDotNet.Test.Core +{ + public class StringInterningTests + { + [Fact] + public void AnchorNameDoesNotInternUniqueValues() + { + var value = UniqueValue("anchor"); + Assert.Null(string.IsInterned(value)); + + var anchor = new AnchorName(value); + + Assert.Equal(value, anchor.Value); + Assert.Null(string.IsInterned(value)); + } + + [Fact] + public void TagNameDoesNotInternUniqueValues() + { + var value = "!" + UniqueValue("tag"); + Assert.Null(string.IsInterned(value)); + + var tag = new TagName(value); + + Assert.Equal(value, tag.Value); + Assert.Null(string.IsInterned(value)); + } + + [Fact] + public void ScalarKeyDoesNotInternUniqueValues() + { + var value = UniqueValue("key"); + Assert.Null(string.IsInterned(value)); + + var scalar = new Scalar( + AnchorName.Empty, + TagName.Empty, + value, + ScalarStyle.Plain, + isPlainImplicit: true, + isQuotedImplicit: false, + Mark.Empty, + Mark.Empty, + isKey: true); + + Assert.True(scalar.IsKey); + Assert.Equal(value, scalar.Value); + Assert.Null(string.IsInterned(value)); + } + + private static string UniqueValue(string prefix) + { + return string.Concat(prefix, "_", Guid.NewGuid().ToString("N")); + } + } +} diff --git a/YamlDotNet.Test/Serialization/MergingParserTests.cs b/YamlDotNet.Test/Serialization/MergingParserTests.cs index b507b6ff7..fc84e197d 100644 --- a/YamlDotNet.Test/Serialization/MergingParserTests.cs +++ b/YamlDotNet.Test/Serialization/MergingParserTests.cs @@ -19,8 +19,12 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +using System; using System.Collections.Generic; using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; using FluentAssertions; using Xunit; using YamlDotNet.Core; @@ -151,5 +155,134 @@ public void MergingParserWithNestedSequence_ShouldNotThrowException() new SerializerBuilder().Build().Serialize(yamlObject!).NormalizeNewLines().Should().Be(etalonMergedYaml); } + + [Fact] + public async Task MergingParserWithMergeKeyBomb_ShouldThrowExceptionWhenTooManyEvents() + { + // Timebox this test to avoid infinite loops in case of bugs. + // 30 seconds should be more than enough for this test to run even on a slow machine, and if it takes longer than that, + // it's likely that the merging parser is not correctly counting events and enforcing the limit. + var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + cancellationTokenSource.Token.Register(() => + { + throw new TimeoutException("The test took too long, likely due to an infinite loop in the merging parser."); + }); + + await Task.Run(() => + { + var sb = new StringBuilder(); + + // Base anchor + sb.AppendLine("a0: &a0"); + sb.AppendLine(" x: 1"); + sb.AppendLine(); + + // Each level merges the previous anchor TWICE (fanout=2), doubling event count + for (int i = 1; i <= 25; i++) + { + sb.AppendLine($"a{i}: &a{i}"); + sb.AppendLine($" <<: *a{i - 1}"); // first merge + sb.AppendLine($" <<: *a{i - 1}"); // second merge + sb.AppendLine(); + } + + sb.AppendLine("final:"); + sb.AppendLine(" <<: *a25"); + + var yaml = sb.ToString(); + var parser = new Parser(new StringReader(yaml)); + var mergingParser = new MergingParser(parser, 1000); + try + { + while (mergingParser.MoveNext()) + { + //move through everything, we're in a timebox so if this takes too long, the cancellation token will trigger and fail the test + } + } + catch (YamlException ex) when (ex.Message.Contains("Too many parsing events")) + { + // Expected exception, test passes + return; + } + catch (Exception ex) + { + throw new Exception($"Unexpected exception", ex); + } + }, cancellationTokenSource.Token); + } + + [Fact] + public async Task MergingParserWithManySmallMerges_ShouldThrowExceptionWhenCumulativeEventsExceedLimit() + { + // Timebox this test to avoid infinite loops in case of bugs. + // 30 seconds should be more than enough for this test to run even on a slow machine, and if it takes longer than that, + // it's likely that the merging parser is not correctly counting events and enforcing the limit. + var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + cancellationTokenSource.Token.Register(() => + { + throw new TimeoutException("The test took too long, likely due to an infinite loop in the merging parser."); + }); + + await Task.Run(() => + { + var sb = new StringBuilder(); + sb.AppendLine("base: &base"); + for (var i = 0; i < 25; i++) + { + sb.AppendLine($" k{i}: v{i}"); + } + + sb.AppendLine(); + for (var i = 0; i < 35; i++) + { + sb.AppendLine($"entry{i}:"); + sb.AppendLine(" <<: *base"); + sb.AppendLine(); + } + + var parser = new Parser(new StringReader(sb.ToString())); + var mergingParser = new MergingParser(parser, 1000); + + Action parse = () => + { + while (mergingParser.MoveNext()) + { + } + }; + + parse.Should().Throw() + .Where(ex => ex.Message.Contains("Too many parsing events")); + }, cancellationTokenSource.Token); + } + + [Fact] + public void MergingParserWithDeepSingleChain_ShouldParseWithinLimit() + { + const int depth = 200; + var sb = new StringBuilder(); + + sb.AppendLine("a0: &a0"); + sb.AppendLine(" root: value"); + sb.AppendLine(); + + for (var i = 1; i <= depth; i++) + { + sb.AppendLine($"a{i}: &a{i}"); + sb.AppendLine($" <<: *a{i - 1}"); + sb.AppendLine($" level{i}: {i}"); + sb.AppendLine(); + } + + sb.AppendLine("final:"); + sb.AppendLine($" <<: *a{depth}"); + + var parser = new Parser(new StringReader(sb.ToString())); + var mergingParser = new MergingParser(parser, 50000); + var deserializer = new DeserializerBuilder().Build(); + + var yamlObject = deserializer.Deserialize>(mergingParser); + + yamlObject.Should().ContainKey("final"); + } } } diff --git a/YamlDotNet.Test/YamlDotNet.Test.csproj b/YamlDotNet.Test/YamlDotNet.Test.csproj index f1958fe7c..3f53966f9 100644 --- a/YamlDotNet.Test/YamlDotNet.Test.csproj +++ b/YamlDotNet.Test/YamlDotNet.Test.csproj @@ -1,6 +1,7 @@  - net10.0;net8.0;net47 + net10.0;net8.0;net4.7 + net10.0;net8.0 false ..\YamlDotNet.snk true diff --git a/YamlDotNet/Core/AnchorName.cs b/YamlDotNet/Core/AnchorName.cs index c0abb38ba..2d458f199 100644 --- a/YamlDotNet/Core/AnchorName.cs +++ b/YamlDotNet/Core/AnchorName.cs @@ -50,7 +50,7 @@ public AnchorName(string value) $"disallowed characters: []{{}},\nThe value was '{value}'.", nameof(value)); } - this.value = string.Intern(value); + this.value = string.IsInterned(value) ?? value; } public override string ToString() => value ?? "[empty]"; diff --git a/YamlDotNet/Core/Events/Scalar.cs b/YamlDotNet/Core/Events/Scalar.cs index b79191a57..8a869fcd2 100644 --- a/YamlDotNet/Core/Events/Scalar.cs +++ b/YamlDotNet/Core/Events/Scalar.cs @@ -79,11 +79,8 @@ public sealed class Scalar : NodeEvent public Scalar(AnchorName anchor, TagName tag, string value, ScalarStyle style, bool isPlainImplicit, bool isQuotedImplicit, Mark start, Mark end, bool isKey = false) : base(anchor, tag, start, end) { - // In a real client program, keys are likely to be - // interned already. Thus, IsInterned might work - // much better, but you can't really benchmark that. this.Value = isKey ? - string.Intern(value) : + string.IsInterned(value) ?? value : value; this.Style = style; this.IsPlainImplicit = isPlainImplicit; diff --git a/YamlDotNet/Core/MergingParser.cs b/YamlDotNet/Core/MergingParser.cs index 14c59393c..4edccb533 100644 --- a/YamlDotNet/Core/MergingParser.cs +++ b/YamlDotNet/Core/MergingParser.cs @@ -36,13 +36,25 @@ public sealed class MergingParser : IParser private readonly IParser innerParser; private IEnumerator> iterator; private bool merged; + private readonly int maxParsingEvents; public MergingParser(IParser innerParser) + : this(innerParser, 100_000) { + } + + public MergingParser(IParser innerParser, int maxParsingEvents = 100_000) + { + if (maxParsingEvents <= 0) + { + throw new ArgumentOutOfRangeException(nameof(maxParsingEvents), "Max parsing events must be a positive integer."); + } + events = new ParsingEventCollection(); merged = false; iterator = events.GetEnumerator(); this.innerParser = innerParser; + this.maxParsingEvents = maxParsingEvents; } public ParsingEvent? Current => iterator.Current?.Value; @@ -64,7 +76,9 @@ private void Merge() { while (innerParser.MoveNext()) { - events.Add(innerParser.Current!); + var parsingEvent = innerParser.Current!; + events.Add(parsingEvent); + EnsureMaxParsingEventsNotExceeded(parsingEvent); } foreach (var node in events) @@ -126,7 +140,7 @@ private bool HandleAnchorAlias(LinkedListNode node, LinkedListNode { var mergedEvents = GetMappingEvents(anchorAlias.Value); - events.AddAfter(node, mergedEvents); + events.AddAfter(node, mergedEvents, EnsureMaxParsingEventsNotExceeded); events.MarkDeleted(anchorNode); return true; @@ -165,6 +179,14 @@ private IEnumerable GetMappingEvents(AnchorName anchor) .Select(cloner.Clone); } + private void EnsureMaxParsingEventsNotExceeded(ParsingEvent parsingEvent) + { + if (events.Count > maxParsingEvents) + { + throw new YamlException(parsingEvent.Start, parsingEvent.End, $"Too many parsing events. The configured limit of {maxParsingEvents} was exceeded to prevent excessive memory usage."); + } + } + private sealed class ParsingEventCollection : IEnumerable> { private readonly LinkedList events; @@ -178,11 +200,14 @@ public ParsingEventCollection() references = []; } - public void AddAfter(LinkedListNode node, IEnumerable items) + public int Count => events.Count; + + public void AddAfter(LinkedListNode node, IEnumerable items, Action onItemAdded) { foreach (var item in items) { node = events.AddAfter(node, item); + onItemAdded(item); } } diff --git a/YamlDotNet/Core/TagName.cs b/YamlDotNet/Core/TagName.cs index ed90cc7b1..1fa882ada 100644 --- a/YamlDotNet/Core/TagName.cs +++ b/YamlDotNet/Core/TagName.cs @@ -49,7 +49,7 @@ public TagName(string value) throw new ArgumentException("Tag value must not be empty.", nameof(value)); } - this.value = string.Intern(value); + this.value = string.IsInterned(value) ?? value; if (IsGlobal && !Uri.IsWellFormedUriString(value, UriKind.RelativeOrAbsolute)) {