diff --git a/Directory.Packages.props b/Directory.Packages.props index ab56b2c273..c1b272acb9 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -28,6 +28,7 @@ + diff --git a/NuGet.config b/NuGet.config index 0c84111e7f..d155153b83 100644 --- a/NuGet.config +++ b/NuGet.config @@ -12,6 +12,8 @@ + + @@ -26,6 +28,7 @@ + diff --git a/sign.thirdparty.props b/sign.thirdparty.props index bde4907f34..0593a3bc41 100644 --- a/sign.thirdparty.props +++ b/sign.thirdparty.props @@ -1,5 +1,7 @@ + + @@ -16,6 +18,7 @@ + diff --git a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs index 4d4a96e046..8f80d1a28a 100644 --- a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs +++ b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs @@ -20,6 +20,7 @@ using Autofac; using Autofac.Core; using Autofac.Extensions.DependencyInjection; +using Ganss.Xss; using Microsoft.ApplicationInsights.Extensibility; using Microsoft.ApplicationInsights.Extensibility.Implementation; using Microsoft.Extensions.DependencyInjection; @@ -132,6 +133,7 @@ protected override void Load(ContainerBuilder builder) services.AddSingleton(loggerFactory); services.AddSingleton(typeof(ILogger<>), typeof(Logger<>)); + services.AddSingleton(); UrlHelperExtensions.SetConfigurationService(configuration); builder.RegisterType() diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj index a8975893ca..09f3494173 100644 --- a/src/NuGetGallery/NuGetGallery.csproj +++ b/src/NuGetGallery/NuGetGallery.csproj @@ -2211,6 +2211,7 @@ + diff --git a/src/NuGetGallery/Services/MarkdownService.cs b/src/NuGetGallery/Services/MarkdownService.cs index 1483e14064..70c71291f9 100644 --- a/src/NuGetGallery/Services/MarkdownService.cs +++ b/src/NuGetGallery/Services/MarkdownService.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -7,6 +7,7 @@ using System.Web; using CommonMark; using CommonMark.Syntax; +using Ganss.Xss; using Markdig; using Markdig.Extensions.EmphasisExtras; using Markdig.Renderers; @@ -20,19 +21,38 @@ public class MarkdownService : IMarkdownService private static readonly TimeSpan RegexTimeout = TimeSpan.FromMinutes(1); private static readonly Regex EncodedBlockQuotePattern = new Regex("^ {0,3}>", RegexOptions.Multiline, RegexTimeout); private static readonly Regex LinkPattern = new Regex("", RegexOptions.None, RegexTimeout); private static readonly Regex HtmlCommentPattern = new Regex("", RegexOptions.Singleline, RegexTimeout); private static readonly Regex ImageTextPattern = new Regex("!\\[\\]\\(", RegexOptions.Singleline, RegexTimeout); - private static readonly string altTextForImage = "alternate text is missing from this package README image"; + private static readonly string AltTextForImage = "alternate text is missing from this package README image"; private readonly IFeatureFlagService _features; private readonly IImageDomainValidator _imageDomainValidator; + private readonly IHtmlSanitizer _htmlSanitizer; public MarkdownService(IFeatureFlagService features, - IImageDomainValidator imageDomainValidator) + IImageDomainValidator imageDomainValidator, + IHtmlSanitizer htmlSanitizer) { _features = features ?? throw new ArgumentNullException(nameof(features)); _imageDomainValidator = imageDomainValidator ?? throw new ArgumentNullException(nameof(imageDomainValidator)); + _htmlSanitizer = htmlSanitizer ?? throw new ArgumentNullException(nameof(htmlSanitizer)); + SanitizerSettings(); + } + + private void SanitizerSettings() + { + //Configure allowed tags, attributes for the sanitizer + _htmlSanitizer.AllowedAttributes.Add("id"); + _htmlSanitizer.AllowedAttributes.Add("class"); + } + + private string SanitizeText(string input) + { + if (!string.IsNullOrWhiteSpace(input)) + { + return _htmlSanitizer.Sanitize(input); + } + return input; } public RenderedMarkdownResult GetHtmlFromMarkdown(string markdownString) @@ -42,6 +62,7 @@ public RenderedMarkdownResult GetHtmlFromMarkdown(string markdownString) throw new ArgumentNullException(nameof(markdownString)); } + if (_features.IsMarkdigMdRenderingEnabled()) { return GetHtmlFromMarkdownMarkdig(markdownString, 1); @@ -179,7 +200,9 @@ private RenderedMarkdownResult GetHtmlFromMarkdownCommonMark(string markdownStri using (var htmlWriter = new StringWriter()) { CommonMarkConverter.ProcessStage3(document, htmlWriter, settings); - output.Content = LinkPattern.Replace(htmlWriter.ToString(), "$0" + " rel=\"noopener noreferrer nofollow\"").Trim(); + string htmlContent = htmlWriter.ToString(); + htmlContent = SanitizeText(htmlContent); + output.Content = LinkPattern.Replace(htmlContent, "$0" + " rel=\"noopener noreferrer nofollow\"").Trim(); return output; } @@ -197,7 +220,7 @@ private RenderedMarkdownResult GetHtmlFromMarkdownMarkdig(string markdownString, var markdownWithoutComments = HtmlCommentPattern.Replace(markdownString, ""); - var markdownWithImageAlt = ImageTextPattern.Replace(markdownWithoutComments, $"![{altTextForImage}]("); + var markdownWithImageAlt = ImageTextPattern.Replace(markdownWithoutComments, $"![{AltTextForImage}]("); var markdownWithoutBom = markdownWithImageAlt.TrimStart('\ufeff'); @@ -286,10 +309,10 @@ private RenderedMarkdownResult GetHtmlFromMarkdownMarkdig(string markdownString, renderer.Render(document); output.Content = htmlWriter.ToString().Trim(); output.IsMarkdigMdSyntaxHighlightEnabled = _features.IsMarkdigMdSyntaxHighlightEnabled(); - output.Content = JavaScriptPattern.Replace(htmlWriter.ToString(), "").Trim(); + output.Content = SanitizeText(output.Content); return output; } } } -} \ No newline at end of file +} diff --git a/src/NuGetGallery/Web.config b/src/NuGetGallery/Web.config index 86fc8533cc..ccdcc31e30 100644 --- a/src/NuGetGallery/Web.config +++ b/src/NuGetGallery/Web.config @@ -739,6 +739,10 @@ + + + + diff --git a/tests/NuGetGallery.Facts/Services/MarkdownServiceFacts.cs b/tests/NuGetGallery.Facts/Services/MarkdownServiceFacts.cs index 26457ef30d..5fd164a68a 100644 --- a/tests/NuGetGallery.Facts/Services/MarkdownServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/MarkdownServiceFacts.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Ganss.Xss; using Moq; using Xunit; @@ -15,12 +16,14 @@ public class GetReadMeHtmlMethod private readonly MarkdownService _markdownService; private readonly Mock _featureFlagService; private readonly Mock _imageDomainValidator; + private readonly IHtmlSanitizer _htmlSanitizer; public GetReadMeHtmlMethod() { _featureFlagService = new Mock(); _imageDomainValidator = new Mock(); - _markdownService = new MarkdownService(_featureFlagService.Object, _imageDomainValidator.Object); + _htmlSanitizer = new HtmlSanitizer(); + _markdownService = new MarkdownService(_featureFlagService.Object, _imageDomainValidator.Object, _htmlSanitizer); } [Theory] @@ -42,10 +45,10 @@ public void ThrowsArgumentNullExceptionForNullMarkdownString() [Theory] [InlineData("", "

<script>alert('test')</script>

", true)] [InlineData("", "

<script>alert('test')</script>

", false)] - [InlineData("", "

<img src="javascript:alert('test');">

", true)] - [InlineData("", "

<img src="javascript:alert('test');">

", false)] - [InlineData("
", "

<a href="javascript:alert('test');">

", true)] - [InlineData("
", "

<a href="javascript:alert('test');">

", false)] + [InlineData("", "

<img src=\"javascript:alert('test');\">

", true)] + [InlineData("", "

<img src=\"javascript:alert('test');\">

", false)] + [InlineData("
", "

<a href=\"javascript:alert('test');\">

", true)] + [InlineData("
", "

<a href=\"javascript:alert('test');\">

", false)] public void EncodesHtmlInMarkdown(string originalMd, string expectedHtml, bool isMarkdigMdRenderingEnabled) { _featureFlagService.Setup(x => x.IsMarkdigMdRenderingEnabled()).Returns(isMarkdigMdRenderingEnabled); @@ -77,34 +80,34 @@ public void EncodesHtmlInMarkdownWithAdaptiveHeader(string originalMd, string ex [InlineData("\ufeff# Heading with BOM", "

Heading with BOM

", false, true)] [InlineData("\ufeff# Heading with BOM", "

Heading with BOM

", false, false)] [InlineData("- List", "
    \n
  • List
  • \n
", false, true)] - [InlineData("- List", "
    \r\n
  • List
  • \r\n
", false, false)] + [InlineData("- List", "
    \n
  • List
  • \n
", false, false)] [InlineData("This is a paragraph\nwithout a break inside", "

This is a paragraph\nwithout a break inside

", false, true)] - [InlineData("This is a paragraph\r\nwithout a break inside", "

This is a paragraph\r\nwithout a break inside

", false, false)] - [InlineData("soft line break line1 \nline2 \nline3 ", "

soft line break line1
\nline2
\nline3

", false, true)] - [InlineData("soft line break line1 \r\nline2 \r\nline3 ", "

soft line break line1
\r\nline2
\r\nline3

", false, false)] + [InlineData("This is a paragraph\nwithout a break inside", "

This is a paragraph\nwithout a break inside

", false, false)] + [InlineData("soft line break line1 \nline2 \nline3 ", "

soft line break line1
\nline2
\nline3

", false, true)] + [InlineData("soft line break line1 \nline2 \nline3 ", "

soft line break line1
\nline2
\nline3

", false, false)] [InlineData("hard line break line1\n\nline2\n\nline3", "

hard line break line1

\n

line2

\n

line3

", false, true)] - [InlineData("hard line break line1\r\n\r\nline2\r\n\r\nline3", "

hard line break line1

\r\n

line2

\r\n

line3

", false, false)] + [InlineData("hard line break line1\n\nline2\n\nline3", "

hard line break line1

\n

line2

\n

line3

", false, false)] [InlineData("[text](http://www.test.com)", "

text

", false, true)] [InlineData("[text](http://www.test.com)", "

text

", false, false)] [InlineData("[text](javascript:alert('hi'))", "

text

", false, true)] [InlineData("[text](javascript:alert('hi'))", "

text

", false, false)] [InlineData("> Blockquote", "
\n

<text>Blockquote</text>

\n
", false, true)] - [InlineData("> Blockquote", "
\r\n

<text>Blockquote</text>

\r\n
", false, false)] + [InlineData("> Blockquote", "
\n

<text>Blockquote</text>

\n
", false, false)] [InlineData("> > Blockquote", "
\n
\n

<text>Blockquote</text>

\n
\n
", false, true)] - [InlineData("> > Blockquote", "
\r\n

> <text>Blockquote</text>

\r\n
", false, false)] + [InlineData("> > Blockquote", "
\n

> <text>Blockquote</text>

\n
", false, false)] [InlineData("[text](http://www.asp.net)", "

text

", false, true)] [InlineData("[text](http://www.asp.net)", "

text

", false, false)] [InlineData("[text](badurl://www.asp.net)", "

text

", false, true)] [InlineData("[text](badurl://www.asp.net)", "

text

", false, false)] - [InlineData("![image](http://www.asp.net/fake.jpg)", "

\"image\"

", true, true)] - [InlineData("![image](http://www.asp.net/fake.jpg)", "

\"image\"

", true, false)] - [InlineData("![image](https://www.asp.net/fake.jpg)", "

\"image\"

", false, true)] - [InlineData("![image](https://www.asp.net/fake.jpg)", "

\"image\"

", false, false)] - [InlineData("![image](http://www.otherurl.net/fake.jpg)", "

\"image\"

", true, true)] - [InlineData("![image](http://www.otherurl.net/fake.jpg)", "

\"image\"

", true, false)] - [InlineData("![](http://www.otherurl.net/fake.jpg)", "

\"\"

", true, false)] - [InlineData("![](http://www.otherurl.net/fake.jpg)", "

\"alternate

", true, true)] - [InlineData("## License\n\tLicensed under the Apache License, Version 2.0 (the \"License\");", "

License

\n
Licensed under the Apache License, Version 2.0 (the "License");\n
", false, true)] + [InlineData("![image](http://www.asp.net/fake.jpg)", "

\"image\"

", true, true)] + [InlineData("![image](http://www.asp.net/fake.jpg)", "

\"image\"

", true, false)] + [InlineData("![image](https://www.asp.net/fake.jpg)", "

\"image\"

", false, true)] + [InlineData("![image](https://www.asp.net/fake.jpg)", "

\"image\"

", false, false)] + [InlineData("![image](http://www.otherurl.net/fake.jpg)", "

\"image\"

", true, true)] + [InlineData("![image](http://www.otherurl.net/fake.jpg)", "

\"image\"

", true, false)] + [InlineData("![](http://www.otherurl.net/fake.jpg)", "

\"\"

", true, false)] + [InlineData("![](http://www.otherurl.net/fake.jpg)", "

\"alternate

", true, true)] + [InlineData("## License\n\tLicensed under the Apache License, Version 2.0 (the \"License\");", "

License

\n
Licensed under the Apache License, Version 2.0 (the \"License\");\n
", false, true)] public void ConvertsMarkdownToHtml(string originalMd, string expectedHtml, bool imageRewriteExpected, bool isMarkdigMdRenderingEnabled) { _featureFlagService.Setup(x => x.IsMarkdigMdRenderingEnabled()).Returns(isMarkdigMdRenderingEnabled); @@ -115,8 +118,8 @@ public void ConvertsMarkdownToHtml(string originalMd, string expectedHtml, bool } [Theory] - [InlineData(true, "

\"image\"

")] - [InlineData(false, "

\"image\"

")] + [InlineData(true, "

\"image\"

")] + [InlineData(false, "

\"image\"

")] public void ConvertsMarkdownToHtmlWithImageDisaplyed(bool isMarkdigMdRenderingEnabled, string expectedHtml) { string imageUrl = "https://api.codacy.com/example/image.svg"; @@ -131,8 +134,8 @@ public void ConvertsMarkdownToHtmlWithImageDisaplyed(bool isMarkdigMdRenderingEn } [Theory] - [InlineData(true, "

\"image\"

")] - [InlineData(false, "

\"image\"

")] + [InlineData(true, "

\"image\"

")] + [InlineData(false, "

\"image\"

")] public void ConvertsMarkdownToHtmlWithoutImageDisaplyed(bool isMarkdigMdRenderingEnabled, string expectedHtml) { string imageUrl = "https://nuget.org/example/image.svg"; @@ -171,7 +174,7 @@ public void TestToHtmlWithGridTable() +---+---+ "; - var expectedHtml = "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
ab
12
"; + var expectedHtml = "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
ab
12
"; _featureFlagService.Setup(x => x.IsMarkdigMdRenderingEnabled()).Returns(true); var readMeResult = _markdownService.GetHtmlFromMarkdown(originalMd); Assert.Equal(expectedHtml, readMeResult.Content); @@ -198,8 +201,8 @@ public void TestToHtmlWithTaskLists() - [ ] Item3 - Item4"; - var expectedHtml = "
    \n
  • Item1
  • \n
  • " + - " Item2
  • \n
  • " + + var expectedHtml = "
      \n
    • Item1
    • \n
    • " + + " Item2
    • \n
    • " + "Item3
    • \n
    • Item4
    • \n
    "; _featureFlagService.Setup(x => x.IsMarkdigMdRenderingEnabled()).Returns(true); var readMeResult = _markdownService.GetHtmlFromMarkdown(originalMd);