Report a block whose aliases form a cycle or expand too far as unreadable [patch]

Frontmatter.Test/YamlAliasTests.cs

@@ -0,0 +1,70 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Frontmatter.Test;
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+/// <summary>
+/// Regression tests for YAML aliases. An alias inside its own anchor used to overflow the stack, which
+/// killed the process, and nested aliases used to expand exponentially.
+/// </summary>
+[TestClass]
+public class YamlAliasTests
+{
+	// Six levels of ten aliases each expand to a million values.
+	private static readonly string AliasBomb = string.Join("\n",
+		"a: &a [x, x, x, x, x, x, x, x, x, x]",
+		"b: &b [*a, *a, *a, *a, *a, *a, *a, *a, *a, *a]",
+		"c: &c [*b, *b, *b, *b, *b, *b, *b, *b, *b, *b]",
+		"d: &d [*c, *c, *c, *c, *c, *c, *c, *c, *c, *c]",
+		"e: &e [*d, *d, *d, *d, *d, *d, *d, *d, *d, *d]",
+		"f: &f [*e, *e, *e, *e, *e, *e, *e, *e, *e, *e]");
+
+	[TestMethod]
+	public void TryParseYamlObject_SequenceContainsItsOwnAlias_ReturnsFalse()
+	{
+		Assert.IsFalse(YamlSerializer.TryParseYamlObject("a: &x [1, *x]", out Dictionary<string, object>? result));
+		Assert.IsNull(result);
+	}
+
+	[TestMethod]
+	public void TryParseYamlObject_MappingContainsItsOwnAlias_ReturnsFalse()
+	{
+		Assert.IsFalse(YamlSerializer.TryParseYamlObject("a: &x {b: *x}", out Dictionary<string, object>? result));
+		Assert.IsNull(result);
+	}
+
+	[TestMethod]
+	public void TryParseYamlObject_NestedAliasesExpandPastTheLimit_ReturnsFalse() =>
+		Assert.IsFalse(YamlSerializer.TryParseYamlObject(AliasBomb, out _));
+
+	[TestMethod]
+	public void TryParseYamlObject_AliasUsedTwiceWithoutACycle_ExpandsBothUses()
+	{
+		Assert.IsTrue(YamlSerializer.TryParseYamlObject("a: &x [p, q]\nb: *x\nc: [*x, *x]", out Dictionary<string, object>? result));
+
+		CollectionAssert.AreEqual(new object[] { "p", "q" }, ((List<object>)result["a"]).ToArray());
+		CollectionAssert.AreEqual(new object[] { "p", "q" }, ((List<object>)result["b"]).ToArray());
+		Assert.HasCount(2, (List<object>)result["c"]);
+	}
+
+	[TestMethod]
+	public void ExtractFrontmatter_SelfReferencingMapping_ReturnsNull() =>
+		Assert.IsNull(Frontmatter.ExtractFrontmatter("---\na: &x {b: *x}\n---\nbody\n"));
+
+	[TestMethod]
+	public void CombineFrontmatter_SelfReferencingSequence_ReturnsTheDocumentUnchanged()
+	{
+		const string input = "---\na: &x [1, *x]\n---\nbody\n";
+
+		Assert.AreEqual(input, Frontmatter.CombineFrontmatter(input, FrontmatterNaming.AsIs, FrontmatterOrder.AsIs, FrontmatterMergeStrategy.None));
+	}
+
+	[TestMethod]
+	public void CombineFrontmatter_AliasBomb_ReturnsTheDocumentUnchanged()
+	{
+		string input = $"---\n{AliasBomb}\n---\nbody\n";
+
+		Assert.AreEqual(input, Frontmatter.CombineFrontmatter(input));
+	}
+}

Frontmatter/YamlSerializer.cs

@@ -21,6 +21,12 @@
 	/// </summary>
 	private static readonly ConcurrentDictionary<string, Dictionary<string, object>> ParsedYamlCache = new(StringComparer.Ordinal);
 
+	/// <summary>
+	/// The most values one block may expand to once its aliases are resolved. Frontmatter is a few dozen
+	/// values; the limit only stops nested aliases from expanding exponentially.
+	/// </summary>
+	internal const int MaxExpandedValues = 100_000;
+
 	/// <summary>
 	/// Reusable deserializer instance
 	/// </summary>
@@ -45,7 +51,7 @@
 	/// <param name="input">The YAML string to parse.</param>
 	/// <param name="result">When this method returns, contains the deserialized dictionary if parsing succeeded, or null if parsing failed.</param>
 	/// <returns>true if the YAML was successfully parsed; otherwise, false.</returns>
 	public static bool TryParseYamlObject(string input, [NotNullWhen(true)] out Dictionary<string, object>? result)
 	{
 		result = null;
 
@@ -78,6 +84,7 @@
 			}
 
 			result = [];
+			ConversionState state = new();
 
 			// Convert dictionary keys to strings and preserve the first occurrence of duplicate keys
 			foreach (KeyValuePair<object, object> pair in rawData)
@@ -89,7 +96,7 @@
 					if (!result.ContainsKey(key))
 					{
 						// Convert the value to ensure proper type handling
-						result[key] = ConvertValue(pair.Value);
+						result[key] = ConvertValue(pair.Value, state);
 					}
 				}
 			}
@@ -157,19 +164,65 @@
 	/// <summary>
 	/// Converts a deserialized value to the appropriate type.
 	/// </summary>
-	private static object ConvertValue(object? value)
+	/// <remarks>
+	/// YamlDotNet resolves every alias to the same object as its anchor, so an alias inside its own anchor
+	/// makes a cyclic graph and nested aliases make a graph far larger than its text. Copying either one
+	/// recursively would overflow the stack, which cannot be caught, or exhaust memory, so both are
+	/// rejected with an <see cref="InvalidOperationException"/> that reports the block as unreadable.
+	/// </remarks>
+	private static object ConvertValue(object? value, ConversionState state)
 	{
-		return value switch
+		if (++state.ValueCount > MaxExpandedValues)
+		{
+			throw new InvalidOperationException($"The YAML expands to more than {MaxExpandedValues} values.");
+		}
+
+		if (value is not System.Collections.IEnumerable or string)
 		{
 			// A null stays null so it is written back as a null rather than as an empty string. The
 			// dictionaries this feeds are typed as non-nullable for compatibility, so the null is forgiven.
-			null => null!,
+			return value!;
+		}
+
+		if (!state.Ancestors.Add(value))
+		{
+			throw new InvalidOperationException("The YAML contains an alias that refers to itself.");
+		}
+
+		object converted = value switch
+		{
 			Dictionary<object, object> dict => dict.ToDictionary(
 				kvp => kvp.Key?.ToString() ?? string.Empty,
-				kvp => ConvertValue(kvp.Value)),
-			List<object> list => list.Select(ConvertValue).ToList(),
-			System.Collections.IList list => list.Cast<object>().Select(ConvertValue).ToList(),
+				kvp => ConvertValue(kvp.Value, state)),
+			List<object> list => list.Select(item => ConvertValue(item, state)).ToList(),
+			System.Collections.IList list => list.Cast<object>().Select(item => ConvertValue(item, state)).ToList(),
 			_ => value
 		};
+
+		state.Ancestors.Remove(value);
+		return converted;
+	}
+
+	/// <summary>
+	/// Tracks one block's conversion: the containers on the path being converted, to find a cycle, and
+	/// the number of values converted so far, to bound alias expansion.
+	/// </summary>
+	private sealed class ConversionState
+	{
+		public HashSet<object> Ancestors { get; } = new(ReferenceComparer.Instance);
+
+		public int ValueCount { get; set; }
+	}
+
+	/// <summary>
+	/// Compares objects by reference. ReferenceEqualityComparer is not available on .NET Standard.
+	/// </summary>
+	private sealed class ReferenceComparer : IEqualityComparer<object>
+	{
+		public static ReferenceComparer Instance { get; } = new();
+
+		public new bool Equals(object? x, object? y) => ReferenceEquals(x, y);
+
+		public int GetHashCode(object obj) => System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(obj);
 	}
 }