Skip to content

Prevent quadratic DoS in chart number-format bracket parsing#1923

Merged
PrzemyslawKlys merged 1 commit into
masterfrom
fix-quadratic-parsing-dos-vulnerability
Jun 16, 2026
Merged

Prevent quadratic DoS in chart number-format bracket parsing#1923
PrzemyslawKlys merged 1 commit into
masterfrom
fix-quadratic-parsing-dos-vulnerability

Conversation

@PrzemyslawKlys

Copy link
Copy Markdown
Member

Motivation

  • Chart number-format literals read from OpenXML can be attacker-controlled and the previous normalization logic called IndexOf(']') from every unmatched '[', producing O(n^2) CPU for long malicious inputs.
  • The goal is to remove the repeated suffix scans while preserving existing behavior for matched bracket directives and literal output for unmatched brackets.

Description

  • Replace per-'[' forward IndexOf scans with a single-pass tracker (nextClosingBracket) so the literal is walked once and closing brackets are advanced linearly; the change is in OfficeIMO.Drawing/OfficeChartDrawingRenderer.Text.cs in NormalizeDataLabelFormatLiteral.
  • Add a regression unit test FlowDrawing_IgnoresBracketDirectivesInDataLabelNumberFormatsLinearly that verifies matched directives are stripped (e.g. "[Red]$0") and that very long unmatched '[' prefixes are preserved as literals without quadratic cost in OfficeIMO.Tests/Pdf/PdfDocumentChartDrawingTests.cs.
  • No project target frameworks or packaging settings were changed and net472 / net8.0 / other existing targets were preserved.

Testing

  • Repository static checks were run and no whitespace or obvious diff issues were reported.
  • The new unit test was added but could not be executed in this environment because dotnet is not available; the test is included and should be run in CI or a local environment with the repository test matrix (for example: run dotnet test OfficeIMO.Tests/OfficeIMO.Tests.csproj for the relevant target frameworks).

Codex Task

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.33%. Comparing base (20cf306) to head (9ec917b).
⚠️ Report is 20 commits behind head on master.

Files with missing lines Patch % Lines
...ficeIMO.Drawing/OfficeChartDrawingRenderer.Text.cs 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1923      +/-   ##
==========================================
+ Coverage   73.29%   73.33%   +0.03%     
==========================================
  Files        2256     2521     +265     
  Lines      262814   283188   +20374     
  Branches    56792    60585    +3793     
==========================================
+ Hits       192633   207670   +15037     
- Misses      44317    47879    +3562     
- Partials    25864    27639    +1775     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PrzemyslawKlys PrzemyslawKlys merged commit 94adc54 into master Jun 16, 2026
21 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the fix-quadratic-parsing-dos-vulnerability branch June 16, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant