Address security review bounds checks

PrzemyslawKlys · PrzemyslawKlys · commit e508095108a4 · 2026-06-17T12:32:27.000+02:00
diff --git a/OfficeIMO.Html/Rtf/Internal/RtfHtmlReader.Fields.cs b/OfficeIMO.Html/Rtf/Internal/RtfHtmlReader.Fields.cs
@@ -59,8 +59,98 @@ private bool IsHyperlinkFieldAllowed(IElement token, string? instruction) {
                 return AreHyperlinkFieldTargetsAllowed(token, null);
             }
 
+            if (!TryReadHyperlinkInstructionTargets(instruction!, out IReadOnlyList<string> instructionTargets)) {
+                return false;
+            }
+
             var field = new RtfField(instruction!);
-            return AreHyperlinkFieldTargetsAllowed(token, field.HyperlinkField?.Target?.ToString());
+            string? instructionTarget = instructionTargets.Count == 0
+                ? field.HyperlinkField?.Target?.ToString()
+                : instructionTargets[0];
+            return AreHyperlinkFieldTargetsAllowed(token, instructionTarget);
+        }
+
+        private bool TryReadHyperlinkInstructionTargets(string instruction, out IReadOnlyList<string> targets) {
+            targets = Array.Empty<string>();
+            IReadOnlyList<string> tokens = TokenizeRtfFieldInstruction(instruction);
+            if (tokens.Count == 0 || !string.Equals(tokens[0], "HYPERLINK", StringComparison.OrdinalIgnoreCase)) {
+                return true;
+            }
+
+            var values = new List<string>();
+            for (int index = 1; index < tokens.Count; index++) {
+                string token = tokens[index];
+                if (token.Length == 0) {
+                    continue;
+                }
+
+                if (token[0] == '\\') {
+                    if (RtfHyperlinkSwitchConsumesValue(token) && index + 1 < tokens.Count) {
+                        index++;
+                    }
+
+                    continue;
+                }
+
+                values.Add(token);
+            }
+
+            if (values.Count > 1) {
+                _options.AddDiagnostic(
+                    "RtfHtmlFieldHyperlinkRejected",
+                    "RTF hyperlink field instruction contains multiple targets.",
+                    "data-officeimo-rtf-field-instruction");
+                return false;
+            }
+
+            targets = values;
+            return true;
+        }
+
+        private static bool RtfHyperlinkSwitchConsumesValue(string token) =>
+            string.Equals(token, "\\l", StringComparison.OrdinalIgnoreCase) ||
+            string.Equals(token, "\\m", StringComparison.OrdinalIgnoreCase) ||
+            string.Equals(token, "\\n", StringComparison.OrdinalIgnoreCase) ||
+            string.Equals(token, "\\o", StringComparison.OrdinalIgnoreCase) ||
+            string.Equals(token, "\\t", StringComparison.OrdinalIgnoreCase);
+
+        private static IReadOnlyList<string> TokenizeRtfFieldInstruction(string instruction) {
+            var tokens = new List<string>();
+            int index = 0;
+            while (index < instruction.Length) {
+                while (index < instruction.Length && char.IsWhiteSpace(instruction[index])) {
+                    index++;
+                }
+
+                if (index >= instruction.Length) {
+                    break;
+                }
+
+                if (instruction[index] == '"') {
+                    index++;
+                    var quoted = new System.Text.StringBuilder();
+                    while (index < instruction.Length) {
+                        char c = instruction[index++];
+                        if (c == '"') {
+                            break;
+                        }
+
+                        quoted.Append(c);
+                    }
+
+                    tokens.Add(quoted.ToString());
+                    continue;
+                }
+
+                int start = index;
+                while (index < instruction.Length && !char.IsWhiteSpace(instruction[index])) {
+                    index++;
+                }
+
+                tokens.Add(instruction.Substring(start, index - start));
+            }
+
+            return tokens;
         }
 
         private bool AreHyperlinkFieldTargetsAllowed(IElement token, string? instructionTarget) {
diff --git a/OfficeIMO.Pdf/Reading/Font/ToUnicodeCMap.cs b/OfficeIMO.Pdf/Reading/Font/ToUnicodeCMap.cs
@@ -10,6 +10,7 @@ internal sealed class ToUnicodeCMap {
     private readonly Dictionary<string, string> _reverseMap = new(StringComparer.Ordinal);
     private int _maxKeyBytes = 1;
     private int _maxReverseTextLength = 1;
+    private int _processedMappings;
 
     public static bool TryParse(byte[] data, out ToUnicodeCMap? cmap) {
         try {
@@ -25,20 +26,36 @@ private void Parse(string s) {
         var bfchar = new Regex(@"<(?<src>[0-9A-Fa-f]+)>\s+<(?<dst>[0-9A-Fa-f]+)>");
         foreach (Match section in Regex.Matches(s, @"beginbfchar([\s\S]*?)endbfchar", RegexOptions.IgnoreCase)) {
             foreach (Match m in bfchar.Matches(section.Groups[1].Value)) {
+                if (HasReachedMappingLimit()) {
+                    break;
+                }
+
                 AddMap(m.Groups["src"].Value, m.Groups["dst"].Value);
             }
+
+            if (HasReachedMappingLimit()) {
+                break;
+            }
         }
         // Handle beginbfrange / endbfrange, sequential mapping
         var bfrangeLine = new Regex(@"<(?<from>[0-9A-Fa-f]+)>\s+<(?<to>[0-9A-Fa-f]+)>\s+<(?<dst>[0-9A-Fa-f]+)>");
         foreach (Match section in Regex.Matches(s, @"beginbfrange([\s\S]*?)endbfrange", RegexOptions.IgnoreCase)) {
+            if (HasReachedMappingLimit()) {
+                break;
+            }
+
             string body = section.Groups[1].Value;
             string sequentialBody = Regex.Replace(body, @"<(?<from>[0-9A-Fa-f]+)>\s+<(?<to>[0-9A-Fa-f]+)>\s+\[(?<dsts>[\s\S]*?)\]", string.Empty, RegexOptions.IgnoreCase);
             foreach (Match m in bfrangeLine.Matches(sequentialBody)) {
+                if (HasReachedMappingLimit()) {
+                    break;
+                }
+
                 int from = Convert.ToInt32(m.Groups["from"].Value, 16);
                 int to = Convert.ToInt32(m.Groups["to"].Value, 16);
                 int dst = Convert.ToInt32(m.Groups["dst"].Value, 16);
                 int rangeLength = to >= from ? to - from + 1 : 0;
-                if (rangeLength <= 0 || rangeLength > MaxRangeMappings || _map.Count + rangeLength > MaxMappings) {
+                if (rangeLength <= 0 || rangeLength > MaxRangeMappings || _processedMappings + rangeLength > MaxMappings) {
                     continue;
                 }
 
@@ -51,6 +68,10 @@ private void Parse(string s) {
             }
 
             foreach (Match m in Regex.Matches(body, @"<(?<from>[0-9A-Fa-f]+)>\s+<(?<to>[0-9A-Fa-f]+)>\s+\[(?<dsts>[\s\S]*?)\]", RegexOptions.IgnoreCase)) {
+                if (HasReachedMappingLimit()) {
+                    break;
+                }
+
                 int from = Convert.ToInt32(m.Groups["from"].Value, 16);
                 int to = Convert.ToInt32(m.Groups["to"].Value, 16);
                 int keyBytes = m.Groups["from"].Value.Length / 2;
@@ -60,7 +81,7 @@ private void Parse(string s) {
                         break;
                     }
 
-                    if (_map.Count >= MaxMappings || code - from >= MaxRangeMappings) {
+                    if (HasReachedMappingLimit() || code - from >= MaxRangeMappings) {
                         break;
                     }
 
@@ -73,10 +94,11 @@ private void Parse(string s) {
     }
 
     private void AddMap(string srcHex, string dstHex) {
-        if (_map.Count >= MaxMappings) {
+        if (HasReachedMappingLimit()) {
             return;
         }
 
+        _processedMappings++;
         srcHex = RemoveHexWhitespace(srcHex);
         dstHex = RemoveHexWhitespace(dstHex);
         if (srcHex.Length % 2 != 0) srcHex = "0" + srcHex;
@@ -94,6 +116,8 @@ private void AddMap(string srcHex, string dstHex) {
         }
     }
 
+    private bool HasReachedMappingLimit() => _processedMappings >= MaxMappings;
+
     private static string RemoveHexWhitespace(string value) {
         bool hasWhitespace = false;
         for (int i = 0; i < value.Length; i++) {
diff --git a/OfficeIMO.Tests/Pdf/PdfDocumentChartDrawingTests.cs b/OfficeIMO.Tests/Pdf/PdfDocumentChartDrawingTests.cs
@@ -178,6 +178,23 @@ public void WordChartSeriesExtraction_ExtendsCategoriesAcrossAllSeries() {
         Assert.Equal(new[] { "Q1", "Q2", "Q3", "Q4" }, categories);
     }
 
+    [Fact]
+    public void WordChartSeriesExtraction_RejectsTooManySeriesBeforeOrdering() {
+        var chart = new BarChart(
+            new BarDirection { Val = BarDirectionValues.Column },
+            new BarGrouping { Val = BarGroupingValues.Clustered });
+        for (uint index = 0; index < 257; index++) {
+            chart.Append(CreateBarSeries(index, new[] { "Q1" }, new[] { 1D }));
+        }
+
+        MethodInfo method = typeof(WordPdfConverterExtensions).GetMethod("ExtractNativeWordChartSeries", BindingFlags.NonPublic | BindingFlags.Static)!;
+        object?[] args = { new Chart(), chart, OfficeChartKind.ColumnClustered, new Dictionary<A.SchemeColorValues, OfficeColor>(), null };
+
+        TargetInvocationException exception = Assert.Throws<TargetInvocationException>(() => method.Invoke(null, args));
+
+        Assert.Contains("maximum supported series count", exception.InnerException?.Message);
+    }
+
     [Fact]
     public void WordChartSeriesExtraction_PreservesNonPiePointFillColors() {
         OfficeColor highlight = OfficeColor.ParseHex("#F76707");
diff --git a/OfficeIMO.Tests/Pdf/PdfFontSecurityTests.cs b/OfficeIMO.Tests/Pdf/PdfFontSecurityTests.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Reflection;
 using System.Text;
 using OfficeIMO.Pdf;
 using Xunit;
@@ -25,6 +26,24 @@ public void ToUnicodeCMap_SkipsOversizedSequentialRanges() {
         Assert.NotEqual("B", cmap.MapBytes(new byte[] { 0x10, 0x00 }));
     }
 
+    [Fact]
+    public void ToUnicodeCMap_CapsDuplicateSourceEntriesByProcessedCount() {
+        var builder = new StringBuilder();
+        builder.AppendLine("beginbfchar");
+        for (int index = 0; index < 70000; index++) {
+            builder.AppendLine("<01> <0041>");
+        }
+
+        builder.AppendLine("endbfchar");
+
+        Assert.True(ToUnicodeCMap.TryParse(Encoding.ASCII.GetBytes(builder.ToString()), out ToUnicodeCMap? cmap));
+        Assert.NotNull(cmap);
+
+        FieldInfo field = typeof(ToUnicodeCMap).GetField("_processedMappings", BindingFlags.NonPublic | BindingFlags.Instance)!;
+        Assert.Equal(65536, (int)field.GetValue(cmap!)!);
+        Assert.Equal("A", cmap!.MapBytes(new byte[] { 0x01 }));
+    }
+
     [Fact]
     public void ResourceResolver_CapsCidWidthRangeExpansion() {
         var page = new PdfDictionary();
diff --git a/OfficeIMO.Tests/Rtf/RtfHtmlFieldTests.cs b/OfficeIMO.Tests/Rtf/RtfHtmlFieldTests.cs
@@ -105,6 +105,25 @@ public void Html_ToRtfDocument_Rejects_Unsafe_Hyperlink_Field_Instruction_Target
         Assert.Equal("data-officeimo-rtf-field-instruction", diagnostic.Source);
     }
 
+    [Fact]
+    public void Html_ToRtfDocument_Rejects_Hyperlink_Field_Instruction_With_Extra_Target() {
+        const string html = "<p><a href=\"https://example.test/\" data-officeimo-rtf-field=\"true\" data-officeimo-rtf-field-instruction=\"HYPERLINK &quot;file:///C:/secret.txt&quot; &quot;https://example.test/&quot;\">Link</a></p>";
+        var options = new HtmlToRtfOptions {
+            UrlPolicy = HtmlUrlPolicy.CreateWebOnlyProfile()
+        };
+
+        RtfDocument document = html.ToRtfDocument(options);
+        RtfParagraph paragraph = Assert.Single(document.Paragraphs);
+        string rtf = document.ToRtf();
+
+        Assert.Empty(paragraph.Inlines.OfType<RtfField>());
+        Assert.Equal("Link", paragraph.ToPlainText());
+        Assert.DoesNotContain("file:///C:/secret.txt", rtf, StringComparison.OrdinalIgnoreCase);
+        HtmlRtfConversionDiagnostic diagnostic = Assert.Single(options.Diagnostics);
+        Assert.Equal("RtfHtmlFieldHyperlinkRejected", diagnostic.Code);
+        Assert.Equal("data-officeimo-rtf-field-instruction", diagnostic.Source);
+    }
+
     [Fact]
     public void Html_ToRtfDocument_Rejects_Unsafe_Hyperlink_Field_Metadata_Target() {
         const string html = "<p><a href=\"https://example.test/\" data-officeimo-rtf-field=\"true\" data-officeimo-rtf-field-instruction=\"HYPERLINK &quot;https://example.test/&quot;\" data-officeimo-rtf-field-hyperlink=\"javascript:alert(1)\">Link</a></p>";
diff --git a/OfficeIMO.Word.Pdf/WordPdfConverterExtensions.Native.Charts.cs b/OfficeIMO.Word.Pdf/WordPdfConverterExtensions.Native.Charts.cs
@@ -439,13 +439,23 @@ private static bool TryParseNativeWordChartNumber(string? text, out double value
             return false;
         }
 
-        private static IEnumerable<(OpenXmlElement SeriesElement, int OriginalSeriesIndex)> GetNativeWordOrderedSeriesElements(OpenXmlElement chartElement) {
-            return chartElement.ChildElements
-                .Where(element => element.LocalName == "ser")
-                .Select((element, index) => (SeriesElement: element, OriginalSeriesIndex: index, Order: GetNativeWordChartSeriesOrder(element, index)))
+        private static IReadOnlyList<(OpenXmlElement SeriesElement, int OriginalSeriesIndex)> GetNativeWordOrderedSeriesElements(OpenXmlElement chartElement) {
+            var series = new List<(OpenXmlElement SeriesElement, int OriginalSeriesIndex, int Order)>();
+            int index = 0;
+            foreach (OpenXmlElement element in chartElement.ChildElements.Where(element => element.LocalName == "ser")) {
+                if (series.Count >= MaxNativeWordChartSeries) {
+                    throw new NativeWordChartLimitException("Word chart cache exceeds the maximum supported series count.");
+                }
+
+                series.Add((element, index, GetNativeWordChartSeriesOrder(element, index)));
+                index++;
+            }
+
+            return series
                 .OrderBy(item => item.Order)
                 .ThenBy(item => item.OriginalSeriesIndex)
-                .Select(item => (item.SeriesElement, item.OriginalSeriesIndex));
+                .Select(item => (item.SeriesElement, item.OriginalSeriesIndex))
+                .ToArray();
         }
 
         private static int GetNativeWordChartSeriesOrder(OpenXmlElement seriesElement, int fallback) {
