diff --git a/CLAUDE.md b/CLAUDE.md index c87c497..fe29c55 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,7 +63,17 @@ URLs follow `/reqif/{header-identifier}//`. The `Spec ### Display-name fallback -`SpecElementWithAttributesExtensions.ExtractDisplayName` defines the precedence used everywhere a spec element is shown: `LongName` → first `AttributeValue` (XHTML rendered as `MarkupString`) → `Identifier`. Do not reinvent this in individual components. +`SpecElementWithAttributesExtensions.ExtractDisplayName` defines the precedence used everywhere a spec element is shown: `LongName` → first `AttributeValue` → `Identifier`. It returns a safe `MarkupString`: text is encoded, XHTML is sanitized, and it falls back to the identifier when nothing visible is left. `ExtractDisplayText` follows the same order as plain text, for labels and menus. Do not reinvent this in individual components. + +### Rendering content from a ReqIF file + +Everything in an uploaded file is untrusted. Never wrap file content in `new MarkupString(...)` or pass it to `AddMarkupContent` directly: + +- XHTML (for example `AttributeValueXHTML.TheValue`): `SafeMarkup.FromXhtml` (`Utilities/SafeMarkup.cs`), which removes namespace prefixes and sanitizes against an allow-list of the XHTML modules ReqIF uses. +- Plain text: render it as a string (`@text`), which Razor encodes, or use `SafeMarkup.FromText`. +- Text for labels: `SafeMarkup.ToPlainText`, or `ExtractDisplayText`. Do not use ReqIFSharp's `ExtractUnformattedTextFromValue` for display: it throws on values that are not well-formed XML. + +`TestData/xss-attempts.reqif` holds script injection attempts; render new views of file content against it (see `XssAttemptsTestFixture`). ### Layout / UI stack diff --git a/reqifviewer.Tests/Components/RelationMatrixComponentTestFixture.cs b/reqifviewer.Tests/Components/RelationMatrixComponentTestFixture.cs index 19d0727..5f5cc13 100644 --- a/reqifviewer.Tests/Components/RelationMatrixComponentTestFixture.cs +++ b/reqifviewer.Tests/Components/RelationMatrixComponentTestFixture.cs @@ -205,5 +205,45 @@ public void Verify_that_component_fits_the_wrapper_to_the_viewport_when_a_matrix Assert.That(fit.Arguments[0], Is.InstanceOf(), "fit must target the component's wrapper ElementReference"); } + + private IRenderedComponent RenderMatrixFor(SpecObject rowObject) + { + var relation = this.reqIf.CoreContent.SpecRelations.First(r => r.Source != null && r.Target != null && r.Source.Type == rowObject.Type); + + var renderer = this.context.RenderComponent(p => p + .Add(x => x.ReqIf, this.reqIf) + .Add(x => x.RowType, rowObject.Type) + .Add(x => x.ColumnType, relation.Target.Type) + .Add(x => x.RelationType, relation.Type) + .Add(x => x.ShowOnlyRelated, false)); + + renderer.WaitForState(() => !renderer.Instance.IsBusy && renderer.Instance.VisibleRows.Count > 0); + return renderer; + } + + [Test] + public void Verify_that_labels_show_plain_text_for_objects_named_by_an_XHTML_value() + { + // No long name; its first value is XHTML: "Stakeholder Requirements" + var specObject = this.reqIf.CoreContent.SpecObjects.Single(x => x.Identifier == "_o7scQ6dbEeafNduaIhMwQg"); + + var renderer = this.RenderMatrixFor(specObject); + + Assert.That(renderer.Instance.RowLabels[specObject.Identifier], Is.EqualTo("Stakeholder Requirements")); + Assert.That(renderer.Instance.RowLabels.Values, Has.None.Contains("xhtml:")); + Assert.That(renderer.Instance.ColumnLabels.Values, Has.None.Contains("xhtml:")); + } + + [Test] + public void Verify_that_a_text_only_XHTML_value_does_not_break_the_matrix() + { + var specObject = this.reqIf.CoreContent.SpecObjects.Single(x => x.Identifier == "_o7scQ6dbEeafNduaIhMwQg"); + var xhtmlValue = specObject.Values.OfType().First(); + xhtmlValue.TheValue = "plain text without an element"; + + var renderer = this.RenderMatrixFor(specObject); + + Assert.That(renderer.Instance.RowLabels[specObject.Identifier], Is.EqualTo("plain text without an element")); + } } } diff --git a/reqifviewer.Tests/Components/XssAttemptsTestFixture.cs b/reqifviewer.Tests/Components/XssAttemptsTestFixture.cs new file mode 100644 index 0000000..9d88980 --- /dev/null +++ b/reqifviewer.Tests/Components/XssAttemptsTestFixture.cs @@ -0,0 +1,273 @@ +// ------------------------------------------------------------------------------------------------- +// +// +// Copyright 2021-2026 Starion Group S.A. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// +// ------------------------------------------------------------------------------------------------- + +namespace ReqifViewer.Tests.Components +{ + using System.IO; + using System.Linq; + + using Bunit; + + using Microsoft.Extensions.DependencyInjection; + + using Moq; + + using NUnit.Framework; + + using ReqIFSharp; + + using reqifviewer.Components; + + using TestContext = Bunit.TestContext; + + /// + /// Renders the content of TestData/xss-attempts.reqif, a ReqIF file whose names and texts contain script + /// injection attempts, and checks that nothing executable reaches the page + /// + [TestFixture] + public class XssAttemptsTestFixture + { + private TestContext context; + + private ReqIF reqIf; + + [SetUp] + public void SetUp() + { + this.context = new TestContext(); + this.context.JSInterop.Mode = JSRuntimeMode.Loose; + + var path = Path.Combine(NUnit.Framework.TestContext.CurrentContext.TestDirectory, "TestData", "xss-attempts.reqif"); + this.reqIf = new ReqIFDeserializer().Deserialize(path).Single(); + } + + [TearDown] + public void TearDown() + { + this.context.Dispose(); + } + + /// + /// Checks the rendered DOM of the grid cells, where the content of the file is shown: no executable + /// elements, no event handler attributes, no javascript: links and no fixed positioning. Markup that is + /// shown as text is fine. The grid's own buttons (filter, pager) are outside the cells. + /// + private static void AssertNothingExecutable(IRenderedComponent renderer) where TComponent : Microsoft.AspNetCore.Components.IComponent + { + var forbiddenElements = new[] { "script", "img", "svg", "iframe", "object", "embed", "form", "style" }; + + var cells = renderer.FindAll("td"); + Assert.That(cells, Is.Not.Empty, "no grid cells rendered"); + + foreach (var element in cells.Concat(cells.SelectMany(cell => cell.QuerySelectorAll("*")))) + { + var name = element.LocalName; + + Assert.That(forbiddenElements, Does.Not.Contain(name), $"<{name}> element rendered"); + + foreach (var attribute in element.Attributes) + { + Assert.That(attribute.Name.StartsWith("on", System.StringComparison.OrdinalIgnoreCase), Is.False, $"{attribute.Name} attribute on <{name}>"); + Assert.That(attribute.Value, Does.Not.Contain("javascript:").IgnoreCase, $"javascript: in {attribute.Name} on <{name}>"); + + if (attribute.Name == "style") + { + Assert.That(attribute.Value, Does.Not.Contain("position").IgnoreCase, $"positioning style on <{name}>"); + } + } + } + } + + [Test] + public void Verify_that_attribute_values_from_the_file_render_without_executable_content() + { + foreach (var specObject in this.reqIf.CoreContent.SpecObjects) + { + var renderer = this.context.RenderComponent(parameters => parameters.Add(x => x.AttributeValues, specObject.Values)); + + AssertNothingExecutable(renderer); + } + } + + [Test] + public void Verify_that_display_names_from_the_file_render_without_executable_content() + { + var renderer = this.context.RenderComponent(parameters => parameters.Add(x => x.SpecObjects, this.reqIf.CoreContent.SpecObjects)); + + AssertNothingExecutable(renderer); + + // The markup in the long names is shown as text + Assert.That(renderer.Markup, Does.Contain("<img src=x onerror=")); + } + + [Test] + public void Verify_that_legitimate_formatting_still_renders() + { + var legitimate = this.reqIf.CoreContent.SpecObjects.Single(x => x.LongName == "Legitimate formatting"); + + var renderer = this.context.RenderComponent(parameters => parameters.Add(x => x.AttributeValues, legitimate.Values)); + + Assert.That(renderer.Markup, Does.Contain("brew")); + Assert.That(renderer.Markup, Does.Contain("a")); + Assert.That(renderer.Markup, Does.Contain("a safe link")); + } + + /// + /// Checks a part of the rendered DOM that only contains file content: no executable elements, no event + /// handler attributes and no javascript: urls + /// + private static void AssertNothingExecutableIn(AngleSharp.Dom.IElement root) + { + var forbiddenElements = new[] { "script", "img", "svg", "iframe", "object", "embed", "form", "style" }; + + foreach (var element in new[] { root }.Concat(root.QuerySelectorAll("*"))) + { + Assert.That(forbiddenElements, Does.Not.Contain(element.LocalName), $"<{element.LocalName}> element rendered"); + + foreach (var attribute in element.Attributes) + { + Assert.That(attribute.Name.StartsWith("on", System.StringComparison.OrdinalIgnoreCase), Is.False, $"{attribute.Name} attribute on <{element.LocalName}>"); + Assert.That(attribute.Value, Does.Not.Contain("javascript:").IgnoreCase, $"javascript: in {attribute.Name} on <{element.LocalName}>"); + } + } + } + + private Moq.Mock RegisterLoaderService() + { + var loader = new Moq.Mock(); + loader.Setup(x => x.ReqIFData).Returns([this.reqIf]); + loader.Setup(x => x.QueryDataAsync(Moq.It.IsAny(), Moq.It.IsAny())) + .ReturnsAsync("data:image/png;base64,iVBORw0KGgo="); + this.context.Services.AddSingleton(loader.Object); + return loader; + } + + [TestCase("Event handler on an image", "Image:")] + [TestCase("Script element", "Before")] + [TestCase("SVG onload", "SVG")] + public void Verify_that_the_selected_XHTML_value_renders_without_executable_content(string longName, string expectedText) + { + var specObject = this.reqIf.CoreContent.SpecObjects.Single(x => x.LongName == longName); + + var renderer = this.context.RenderComponent(parameters => parameters.Add(x => x.AttributeValues, specObject.Values)); + + // Select the row of the XHTML value: its value cell is the second cell of the row + var row = renderer.FindAll("tbody tr").Single(tr => tr.QuerySelectorAll("td").Length > 1 && tr.QuerySelectorAll("td")[1].TextContent.Contains(expectedText)); + row.QuerySelectorAll("td")[1].Click(); + + var card = renderer.FindAll(".rz-card").Single(x => x.TextContent.Contains("Selected Value:")); + + Assert.That(card.TextContent, Does.Contain(expectedText)); + AssertNothingExecutableIn(card); + } + + [Test] + public void Verify_that_a_selected_string_value_is_shown_as_text() + { + var specObject = this.reqIf.CoreContent.SpecObjects.Single(x => x.Identifier == "_so-7"); + + var renderer = this.context.RenderComponent(parameters => parameters.Add(x => x.AttributeValues, specObject.Values)); + + var row = renderer.FindAll("tbody tr").Single(tr => tr.QuerySelectorAll("td").Length > 1 && tr.QuerySelectorAll("td")[1].TextContent.Contains("" }); + Assert.That(withString.ExtractDisplayName().Value, Is.EqualTo("<script>alert(1)</script>")); } [Test] @@ -105,5 +123,43 @@ public void Verify_that_ExtractDisplayText_collapses_whitespace() Assert.That(specObject.ExtractDisplayText(), Is.EqualTo("First line bold")); } + + [Test] + public void Verify_that_ExtractDisplayText_handles_XHTML_values_that_are_not_well_formed() + { + // Plain text without a root element: ReqIFSharp's ExtractUnformattedTextFromValue throws on this + var textOnly = new SpecObject { Identifier = "object-1" }; + textOnly.Values.Add(new AttributeValueXHTML { TheValue = "plain text without an element" }); + Assert.That(textOnly.ExtractDisplayText(), Is.EqualTo("plain text without an element")); + + var twoParagraphs = new SpecObject { Identifier = "object-2" }; + twoParagraphs.Values.Add(new AttributeValueXHTML { TheValue = "firstsecond" }); + Assert.That(twoParagraphs.ExtractDisplayText(), Is.EqualTo("first second")); + } + + [Test] + public void Verify_that_ExtractDisplayName_falls_back_to_the_identifier_when_nothing_visible_is_left() + { + var emptyLongNameAndNullValue = new SpecObject { Identifier = "object-1", LongName = "" }; + emptyLongNameAndNullValue.Values.Add(new AttributeValueString { TheValue = null }); + Assert.That(emptyLongNameAndNullValue.ExtractDisplayName().Value, Is.EqualTo("object-1")); + + var onlyScript = new SpecObject { Identifier = "object-2" }; + onlyScript.Values.Add(new AttributeValueXHTML { TheValue = "alert(1)" }); + Assert.That(onlyScript.ExtractDisplayName().Value, Is.EqualTo("object-2")); + + var emptyString = new SpecObject { Identifier = "object-3" }; + emptyString.Values.Add(new AttributeValueString { TheValue = " " }); + Assert.That(emptyString.ExtractDisplayName().Value, Is.EqualTo("object-3")); + } + + [Test] + public void Verify_that_ExtractDisplayName_keeps_safe_formatting_of_a_text_only_XHTML_value() + { + var textOnly = new SpecObject { Identifier = "object-1" }; + textOnly.Values.Add(new AttributeValueXHTML { TheValue = "plain & simple" }); + + Assert.That(textOnly.ExtractDisplayName().Value, Is.EqualTo("plain & simple")); + } } } diff --git a/reqifviewer.Tests/TestData/xss-attempts.reqif b/reqifviewer.Tests/TestData/xss-attempts.reqif new file mode 100644 index 0000000..6a2858e --- /dev/null +++ b/reqifviewer.Tests/TestData/xss-attempts.reqif @@ -0,0 +1,227 @@ + + + + + 2026-10-18T12:00:00Z + reqifviewer tests + 1.0 + reqifviewer tests + XSS attempts <img src=x onerror="window.__xss=(window.__xss||0)+1"> + + + + + + + + + + + + + + _dt-string + + + + + _dt-xhtml + + + + + + + + + + + + _ad-id + + + + + _ad-text + + The machine shall brew coffee.aba safe link + + + + _sot + + + + + + + _ad-id + + + + + _ad-text + + Beforewindow.__xss=(window.__xss||0)+1 after + + + + _sot + + + + + + + _ad-id + + + + + _ad-text + + Image: + + + + _sot + + + + + + + _ad-id + + + + + _ad-text + + click me + + + + _sot + + + + + + + _ad-id + + + + + _ad-text + + SVG + + + + _sot + + + + + + + _ad-id + + + + + _ad-text + + Overlay + + + + _sot + + + + + + + _ad-id + + + + + _ad-text + + The long name and the ID contain markup + + + + _sot + + + + + + + _ad-text + + Unnamed + + + + _sot + + + + + + + _st + + + + + _so-1 + + + + + _so-2 + + + + + _so-3 + + + + + _so-4 + + + + + _so-5 + + + + + _so-6 + + + + + _so-7 + + + + + _so-unnamed + + + + + + + + \ No newline at end of file diff --git a/reqifviewer.Tests/Utilities/SafeMarkupTestFixture.cs b/reqifviewer.Tests/Utilities/SafeMarkupTestFixture.cs new file mode 100644 index 0000000..1154315 --- /dev/null +++ b/reqifviewer.Tests/Utilities/SafeMarkupTestFixture.cs @@ -0,0 +1,207 @@ +// ------------------------------------------------------------------------------------------------- +// +// +// Copyright 2021-2026 Starion Group S.A. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// +// ------------------------------------------------------------------------------------------------- + +namespace ReqifViewer.Tests.Utilities +{ + using NUnit.Framework; + + using reqifviewer.Utilities; + + /// + /// Suite of tests for the class + /// + [TestFixture] + public class SafeMarkupTestFixture + { + private const string Xhtml = "xmlns:xhtml=\"http://www.w3.org/1999/xhtml\""; + + [Test] + public void Verify_that_ReqIF_formatting_is_kept_without_namespace_prefixes() + { + var markup = SafeMarkup.FromXhtml($"The machine shall brew coffee.one"); + + Assert.That(markup.Value, Is.EqualTo("

The machine shall brew coffee.

  • one
")); + } + + [Test] + public void Verify_that_tables_and_their_spans_are_kept() + { + var markup = SafeMarkup.FromXhtml($"Headab"); + + Assert.That(markup.Value, Does.Contain("Head")); + Assert.That(markup.Value, Does.Contain("ab")); + } + + [TestCase("

default namespace

", "

default namespace

")] + [TestCase("other prefix", "

other prefix

")] + public void Verify_that_other_namespace_forms_are_handled(string xhtml, string expected) + { + Assert.That(SafeMarkup.FromXhtml(xhtml).Value, Is.EqualTo(expected)); + } + + [Test] + public void Verify_that_scripts_are_removed_with_their_content() + { + var markup = SafeMarkup.FromXhtml($"beforealert('xss')after"); + + Assert.That(markup.Value, Is.EqualTo("
beforeafter
")); + } + + [TestCase("")] + [TestCase("text")] + [TestCase("alert(1)")] + [TestCase("")] + [TestCase("")] + [TestCase("link")] + [TestCase("link")] + [TestCase("link")] + [TestCase("text")] + [TestCase("")] + [TestCase("text")] + [TestCase("text")] + [TestCase("entity encoded")] + [TestCase("leading space")] + [TestCase("quote")] + public void Verify_that_executable_content_is_removed(string attack) + { + var value = SafeMarkup.FromXhtml($"{attack}").Value.ToLowerInvariant(); + + Assert.That(value, Does.Not.Contain("reqif.academy"); + + Assert.That(markup.Value, Is.EqualTo("reqif.academy")); + } + + [Test] + public void Verify_that_classes_and_positioning_styles_are_removed_and_formatting_styles_kept() + { + var markup = SafeMarkup.FromXhtml($"text"); + + Assert.That(markup.Value, Does.Not.Contain("class=")); + Assert.That(markup.Value, Does.Not.Contain("id=")); + Assert.That(markup.Value, Does.Not.Contain("position")); + Assert.That(markup.Value, Does.Not.Contain("z-index")); + Assert.That(markup.Value, Does.Contain("color: rgba(255, 0, 0, 1)").Or.Contain("color: red")); + Assert.That(markup.Value, Does.Contain("font-weight: bold")); + } + + [Test] + public void Verify_that_the_fallback_text_of_objects_is_kept() + { + var markup = SafeMarkup.FromXhtml($"Figure 1"); + + Assert.That(markup.Value, Is.EqualTo("
Figure 1
")); + } + + [Test] + public void Verify_that_escaped_markup_stays_text() + { + var markup = SafeMarkup.FromXhtml($"<script>alert(1)</script>"); + + Assert.That(markup.Value, Is.EqualTo("

<script>alert(1)</script>

")); + } + + [TestCase(null, "")] + [TestCase("", "")] + [TestCase(" ", "")] + public void Verify_that_empty_XHTML_gives_empty_markup(string xhtml, string expected) + { + Assert.That(SafeMarkup.FromXhtml(xhtml).Value, Is.EqualTo(expected)); + } + + [TestCase("", "<img src=x onerror=alert(1)>")] + [TestCase("Brakes & ", "Brakes & <steering>")] + [TestCase(null, "")] + public void Verify_that_text_is_encoded(string text, string expected) + { + Assert.That(SafeMarkup.FromText(text).Value, Is.EqualTo(expected)); + } + + [TestCase("style", "body { display: none }")] + [TestCase("noscript", "no script")] + [TestCase("textarea", "typed text")] + [TestCase("title", "a title")] + [TestCase("button", "Click")] + [TestCase("svg", "drawing text")] + public void Verify_that_non_text_elements_are_removed_with_their_content(string element, string content) + { + var markup = SafeMarkup.FromXhtml($"before{content}after"); + + Assert.That(markup.Value, Is.EqualTo("
beforeafter
")); + } + + [TestCase("x", "
  1. x
")] + [TestCase("a b", "
a  b
")] + [TestCase("x = 1", "x = 1")] + [TestCase("H2O and x2", "

H2O and x2

")] + [TestCase("usdelins", "

usdelins

")] + [TestCase("Chapter", "

Chapter

")] + [TestCase("termdefinition", "
term
definition
")] + [TestCase("a", "
a
")] + [TestCase("quote", "
quote
")] + public void Verify_that_allowed_XHTML_elements_are_kept(string xhtml, string expected) + { + var markup = SafeMarkup.FromXhtml($"{xhtml}"); + + Assert.That(markup.Value, Is.EqualTo($"
{expected}
")); + } + + [TestCase("mailto:a@b.c")] + [TestCase("#section-2")] + [TestCase("other.html")] + [TestCase("/reqif/doc/specobject/req-1")] + public void Verify_that_mailto_and_relative_links_are_kept(string href) + { + // Relative links stay within the viewer: an unknown address shows the not-found page + var markup = SafeMarkup.FromXhtml($"link"); + + Assert.That(markup.Value, Is.EqualTo($"link")); + } + + [TestCase("plain text without an element", "plain text without an element")] + [TestCase("firstsecond", "first second")] + [TestCase("brew a cup in under 30 seconds.", "brew a cup in under 30 seconds.")] + [TestCase("\n First line\n second & third\n", "First line second & third")] + [TestCase("aalert(1)b", "ab")] + [TestCase("Figure 1", "Figure 1")] + [TestCase("alert(1)", "")] + [TestCase(null, "")] + public void Verify_that_ToPlainText_returns_the_visible_text(string xhtml, string expected) + { + Assert.That(SafeMarkup.ToPlainText(xhtml), Is.EqualTo(expected)); + } + } +} diff --git a/reqifviewer.Tests/reqifviewer.Tests.csproj b/reqifviewer.Tests/reqifviewer.Tests.csproj index f1392fe..2c11d1a 100644 --- a/reqifviewer.Tests/reqifviewer.Tests.csproj +++ b/reqifviewer.Tests/reqifviewer.Tests.csproj @@ -42,6 +42,9 @@ Always + + Always + diff --git a/reqifviewer/Components/AttributeValuesComponent.razor b/reqifviewer/Components/AttributeValuesComponent.razor index 86bdb26..3b74841 100644 --- a/reqifviewer/Components/AttributeValuesComponent.razor +++ b/reqifviewer/Components/AttributeValuesComponent.razor @@ -18,6 +18,7 @@ @using ReqIFSharp; @using ReqIFSharp.Extensions.ReqIFExtensions @using ReqifViewer.ReqIFExtensions +@using reqifviewer.Utilities @using Serilog @@ -38,7 +39,7 @@ { - Selected Value: @(new MarkupString(selectedAttributeValue.ObjectValue.ToString())) + Selected Value: @SafeMarkup.FromXhtml(((AttributeValueXHTML)selectedAttributeValue).TheValue) } @@ -64,7 +65,7 @@ @@ -89,7 +89,7 @@ else } else { - @(new MarkupString(specRelation.Target.ExtractDisplayName().ToString())) + @(specRelation.Target.ExtractDisplayName()) } @@ -110,7 +110,7 @@ else

- @(new MarkupString (payload.Item1.Owner?.ExtractUnformattedTextFromValue())) + @(payload.Item1.Owner?.ExtractUnformattedTextFromValue())
diff --git a/reqifviewer/Pages/SpecRelation/SpecRelationDetailsPage.razor b/reqifviewer/Pages/SpecRelation/SpecRelationDetailsPage.razor index 72a7f57..55a3371 100644 --- a/reqifviewer/Pages/SpecRelation/SpecRelationDetailsPage.razor +++ b/reqifviewer/Pages/SpecRelation/SpecRelationDetailsPage.razor @@ -36,7 +36,7 @@ else { - @(new MarkupString(this.specRelation?.ExtractDisplayName().ToString())) + @(this.specRelation?.ExtractDisplayName()) diff --git a/reqifviewer/Pages/SpecRelation/SpecRelationPage.razor b/reqifviewer/Pages/SpecRelation/SpecRelationPage.razor index b5652cf..8fc2655 100644 --- a/reqifviewer/Pages/SpecRelation/SpecRelationPage.razor +++ b/reqifviewer/Pages/SpecRelation/SpecRelationPage.razor @@ -38,21 +38,21 @@ else diff --git a/reqifviewer/Pages/Specification/SpecificationDetailsPage.razor b/reqifviewer/Pages/Specification/SpecificationDetailsPage.razor index 7fcd3ec..b857f27 100644 --- a/reqifviewer/Pages/Specification/SpecificationDetailsPage.razor +++ b/reqifviewer/Pages/Specification/SpecificationDetailsPage.razor @@ -34,7 +34,7 @@ else { - @(new MarkupString(this.specification?.ExtractDisplayName().ToString())) + @(this.specification?.ExtractDisplayName()) @@ -160,17 +160,7 @@ else var specHierarchy = context.Value as SpecHierarchy; if (specHierarchy != null) { - var displayObject = specHierarchy.Object.ExtractDisplayName(); - - if (displayObject is MarkupString) - { - var markupString = (MarkupString)displayObject; - builder.AddMarkupContent(2, markupString.Value); - } - else - { - builder.AddContent(2, displayObject); - } + builder.AddMarkupContent(2, specHierarchy.Object.ExtractDisplayName().Value); } else { diff --git a/reqifviewer/Pages/Specification/SpecificationPage.razor b/reqifviewer/Pages/Specification/SpecificationPage.razor index 9f1fba2..1cbcd40 100644 --- a/reqifviewer/Pages/Specification/SpecificationPage.razor +++ b/reqifviewer/Pages/Specification/SpecificationPage.razor @@ -39,7 +39,7 @@ else diff --git a/reqifviewer/ReqIFExtensions/SpecElementWithAttributesExtensions.cs b/reqifviewer/ReqIFExtensions/SpecElementWithAttributesExtensions.cs index dfac309..135b709 100644 --- a/reqifviewer/ReqIFExtensions/SpecElementWithAttributesExtensions.cs +++ b/reqifviewer/ReqIFExtensions/SpecElementWithAttributesExtensions.cs @@ -26,6 +26,8 @@ namespace ReqifViewer.ReqIFExtensions using Microsoft.AspNetCore.Components; using ReqIFSharp; + + using reqifviewer.Utilities; /// /// The class provides a number of extension methods for the class @@ -39,39 +41,33 @@ public static class SpecElementWithAttributesExtensions /// The subject /// /// - /// an object that represents the name (value to be displayed to the user) + /// the name as a that is safe to render: text is encoded and XHTML is + /// sanitized with /// /// /// 1. The is returned in case it is not empty or null /// 2. The first is returned in case it is not null (order matters, results may be unexpected and are determined on order in the original ReqIF document) /// 3. The is returned in case the is empty or null and no s are present /// - public static object ExtractDisplayName(this SpecElementWithAttributes specElementWithAttributes) + public static MarkupString ExtractDisplayName(this SpecElementWithAttributes specElementWithAttributes) { - if (string.IsNullOrEmpty(specElementWithAttributes.LongName)) + if (!string.IsNullOrEmpty(specElementWithAttributes.LongName)) { - var attributeValue = specElementWithAttributes.Values.FirstOrDefault(); - if (attributeValue != null) - { - if (attributeValue is AttributeValueXHTML attributeValueXhtml) - { - var markupString = new MarkupString(attributeValueXhtml.TheValue); - return markupString; - } - else - { - return attributeValue.ObjectValue.ToString(); - } - } - else - { - return specElementWithAttributes.Identifier; - } + return SafeMarkup.FromText(specElementWithAttributes.LongName); } - else + + var attributeValue = specElementWithAttributes.Values.FirstOrDefault(); + + // An empty value, or XHTML without visible text (for example only a removed script), would give an + // empty, unclickable label: use the identifier instead + if (attributeValue is AttributeValueXHTML attributeValueXhtml && SafeMarkup.ToPlainText(attributeValueXhtml.TheValue).Length > 0) { - return specElementWithAttributes.LongName; + return SafeMarkup.FromXhtml(attributeValueXhtml.TheValue); } + + var text = attributeValue is AttributeValueXHTML ? null : attributeValue?.ObjectValue?.ToString(); + + return SafeMarkup.FromText(string.IsNullOrWhiteSpace(text) ? specElementWithAttributes.Identifier : text); } /// @@ -100,7 +96,7 @@ public static string ExtractDisplayText(this SpecElementWithAttributes specEleme var text = attributeValue switch { null => null, - AttributeValueXHTML attributeValueXhtml => attributeValueXhtml.ExtractUnformattedTextFromValue(), + AttributeValueXHTML attributeValueXhtml => SafeMarkup.ToPlainText(attributeValueXhtml.TheValue), _ => attributeValue.ObjectValue?.ToString() }; diff --git a/reqifviewer/Utilities/SafeMarkup.cs b/reqifviewer/Utilities/SafeMarkup.cs new file mode 100644 index 0000000..7c95f0d --- /dev/null +++ b/reqifviewer/Utilities/SafeMarkup.cs @@ -0,0 +1,213 @@ +// ------------------------------------------------------------------------------------------------- +// +// +// Copyright 2021-2026 Starion Group S.A. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// +// ------------------------------------------------------------------------------------------------- + +namespace reqifviewer.Utilities +{ + using System; + using System.Collections.Generic; + using System.Net; + using System.Text.RegularExpressions; + + using Ganss.Xss; + + using Microsoft.AspNetCore.Components; + + /// + /// Turns content from a ReqIF file into markup that is safe to render. + /// + /// + /// Everything in a ReqIF file is untrusted: the file may come from anyone. Never wrap file content in + /// directly; use for XHTML and + /// for plain text. + /// + public static partial class SafeMarkup + { + /// + /// The elements of the XHTML modules that ReqIF allows (text, hypertext, list, presentation, edit and + /// table), plus u and s, which tools commonly emit. The object module is left out: objects are shown + /// by their fallback content until attachments are rendered as images. + /// + private static readonly string[] AllowedTags = + [ + "abbr", "acronym", "address", "blockquote", "br", "cite", "code", "dfn", "div", "em", "h1", "h2", "h3", + "h4", "h5", "h6", "kbd", "p", "pre", "q", "samp", "span", "strong", "var", + "a", + "dl", "dt", "dd", "ol", "ul", "li", + "b", "big", "hr", "i", "small", "sub", "sup", "tt", "u", "s", + "del", "ins", + "caption", "table", "td", "th", "tr", "col", "colgroup", "tbody", "thead", "tfoot" + ]; + + /// + /// The attributes that are kept. class and id are left out, so file content cannot pick up or + /// interfere with the styles and elements of the application. + /// + private static readonly string[] AllowedAttributes = + [ + "href", "title", "lang", "dir", "style", "abbr", "align", "valign", "colspan", "rowspan", "span", + "headers", "scope", "summary", "cite", "datetime", "start", "type", "width", "height" + ]; + + /// + /// The CSS properties that are kept in style attributes. Positioning and layering properties are left + /// out, so file content cannot cover or restyle the rest of the page. + /// + private static readonly string[] AllowedCssProperties = + [ + "color", "background-color", "font", "font-family", "font-size", "font-style", "font-weight", + "font-variant", "text-decoration", "text-decoration-line", "text-decoration-style", "text-align", + "text-indent", "text-transform", "vertical-align", "letter-spacing", "word-spacing", "line-height", + "white-space", "list-style-type", "list-style-position", + "margin", "margin-top", "margin-right", "margin-bottom", "margin-left", + "padding", "padding-top", "padding-right", "padding-bottom", "padding-left", + "border", "border-top", "border-right", "border-bottom", "border-left", "border-color", + "border-style", "border-width", "border-collapse", "border-spacing", + "width", "height", "min-width", "max-width" + ]; + + /// + /// Elements whose content is not text that should be shown, and which are therefore removed together + /// with their content + /// + private static readonly HashSet RemovedWithContent = new(StringComparer.OrdinalIgnoreCase) + { + "script", "style", "template", "noscript", "iframe", "frame", "frameset", "embed", "applet", + "svg", "math", "head", "title", "textarea", "select", "button" + }; + + /// + /// The configured sanitizer; + /// is safe to call from several threads once configured + /// + private static readonly HtmlSanitizer Sanitizer = CreateSanitizer(); + + /// + /// Creates markup from a plain text value: the text is HTML encoded, so it is shown as text + /// + /// the text, may be null + /// a that renders the text + public static MarkupString FromText(string text) + { + return new MarkupString(WebUtility.HtmlEncode(text ?? string.Empty)); + } + + /// + /// Creates markup from an XHTML value of a ReqIF file: namespace prefixes are removed and everything + /// outside the allow-list (scripts, event handlers, javascript: links, unknown elements, class names, + /// positioning styles) is removed + /// + /// the XHTML, for example the TheValue of an AttributeValueXHTML, may be null + /// a that is safe to render + public static MarkupString FromXhtml(string xhtml) + { + if (string.IsNullOrWhiteSpace(xhtml)) + { + return new MarkupString(string.Empty); + } + + var html = NamespacePrefix().Replace(xhtml, "<$1"); + + return new MarkupString(Sanitizer.Sanitize(html)); + } + + /// + /// Extracts the visible text of an XHTML value of a ReqIF file, for labels, menus and other places that + /// cannot show formatting + /// + /// the XHTML, may be null, plain text or have several top-level elements + /// + /// the text without markup: block elements are separated by a space, whitespace is collapsed and the + /// content of scripts and other removed elements is left out; an empty string when there is no text + /// + /// + /// Unlike ReqIFSharp's ExtractUnformattedTextFromValue this does not parse the value as XML, so it does + /// not throw on values that are not well-formed or have no root element + /// + public static string ToPlainText(string xhtml) + { + var sanitized = FromXhtml(xhtml).Value; + + if (sanitized.Length == 0) + { + return string.Empty; + } + + var spaced = BlockTag().Replace(sanitized, " "); + var text = WebUtility.HtmlDecode(AnyTag().Replace(spaced, string.Empty)); + + return Whitespace().Replace(text, " ").Trim(); + } + + /// + /// Matches the start or end tag of an element that separates text (paragraphs, list items, cells, line breaks) + /// + [GeneratedRegex(@"]*>", RegexOptions.IgnoreCase)] + private static partial Regex BlockTag(); + + /// + /// Matches any tag of the sanitized markup + /// + [GeneratedRegex(@"<[^>]*>")] + private static partial Regex AnyTag(); + + /// + /// Matches a run of whitespace + /// + [GeneratedRegex(@"\s+")] + private static partial Regex Whitespace(); + + /// + /// Matches the namespace prefix of an element name in a start or end tag, for example the "xhtml:" of + /// <xhtml:p> and </xhtml:p> + /// + [GeneratedRegex(@"<(/?)[A-Za-z_][\w.-]*:(?=[A-Za-z_])")] + private static partial Regex NamespacePrefix(); + + /// + /// Creates and configures the + /// + /// the configured + private static HtmlSanitizer CreateSanitizer() + { + var options = new HtmlSanitizerOptions + { + AllowedTags = new HashSet(AllowedTags, StringComparer.OrdinalIgnoreCase), + AllowedAttributes = new HashSet(AllowedAttributes, StringComparer.OrdinalIgnoreCase), + AllowedCssProperties = new HashSet(AllowedCssProperties, StringComparer.OrdinalIgnoreCase), + AllowedSchemes = new HashSet(["http", "https", "mailto"], StringComparer.OrdinalIgnoreCase), + UriAttributes = new HashSet(["href", "cite"], StringComparer.OrdinalIgnoreCase) + }; + + // Keep the text of removed elements, so unknown formatting does not hide requirement text + var sanitizer = new HtmlSanitizer(options) { KeepChildNodes = true }; + + sanitizer.RemovingTag += (_, e) => + { + if (RemovedWithContent.Contains(e.Tag.LocalName)) + { + e.Tag.Remove(); + e.Cancel = true; + } + }; + + return sanitizer; + } + } +} diff --git a/reqifviewer/reqifviewer.csproj b/reqifviewer/reqifviewer.csproj index 8c77f63..bd6845b 100644 --- a/reqifviewer/reqifviewer.csproj +++ b/reqifviewer/reqifviewer.csproj @@ -35,6 +35,7 @@ +