From 876130c3cddd761d0b849a9a6e22a26c730eed1b Mon Sep 17 00:00:00 2001 From: leileizhang Date: Thu, 8 Jan 2026 14:57:10 +0800 Subject: [PATCH] Fix Peek allowing Local File Inclusion (LFI) and RCE (#44601) ## Summary of the Pull Request In PR #44456, we disabled scripts and HTML, but we still want to allow HTML rendering. This pull request introduces significant security improvements and UI enhancements to the file previewer, focusing on safe rendering of files and better user communication when opening external links. The main changes include strict resource filtering to prevent external content loading (and potential XSS attacks), a more informative dialog when opening external URIs, and improved logic for determining how different file types are previewed. **Security enhancements:** * Added strict resource filtering for non-dev file previews in `BrowserControl` to block external HTTP/S requests and limit local file access to the same directory and subdirectories, preventing XSS and unwanted external content loading. * Set the `WEBVIEW2_ADDITIONAL_BROWSER_ARGUMENTS` environment variable to block new web contents for extra security. * Removed disabling of HTML rendering in the Markdown pipeline to allow safe local rendering (now protected by resource filtering). **File preview logic improvements:** * Refactored previewer logic to prioritize file type handling: Markdown, SVG, HTML/HTM, Monaco-supported source code, and fallback types, ensuring correct preview strategy and context menu behavior for each type. * Resource filtering is dynamically applied or removed based on whether the file is a dev/source code file (Monaco editor) or not, ensuring compatibility and security. **User interface enhancements:** * Updated the open URI dialog to include a warning banner and improved messaging, informing users about the risks of opening external links. image ## PR Checklist - [x] Closes: #44600 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed use this file for testing [test-xss-vulnerability.md](https://github.com/user-attachments/files/24486900/test-xss-vulnerability.md) image --- .../FilePreviewCommon/MarkdownHelper.cs | 2 +- .../Controls/BrowserControl.xaml | 15 +++- .../Controls/BrowserControl.xaml.cs | 83 ++++++++++++++++++- .../WebBrowserPreviewer.cs | 41 ++++----- .../peek/Peek.UI/Strings/en-us/Resources.resw | 10 ++- 5 files changed, 127 insertions(+), 24 deletions(-) diff --git a/src/common/FilePreviewCommon/MarkdownHelper.cs b/src/common/FilePreviewCommon/MarkdownHelper.cs index 6573b8b5ef..2003df3340 100644 --- a/src/common/FilePreviewCommon/MarkdownHelper.cs +++ b/src/common/FilePreviewCommon/MarkdownHelper.cs @@ -39,7 +39,7 @@ namespace Microsoft.PowerToys.FilePreviewCommon var softlineBreak = new Markdig.Extensions.Hardlines.SoftlineBreakAsHardlineExtension(); MarkdownPipelineBuilder pipelineBuilder; - pipelineBuilder = new MarkdownPipelineBuilder().UseAdvancedExtensions().UseEmojiAndSmiley().UseYamlFrontMatter().UseMathematics().DisableHtml(); + pipelineBuilder = new MarkdownPipelineBuilder().UseAdvancedExtensions().UseEmojiAndSmiley().UseYamlFrontMatter().UseMathematics(); pipelineBuilder.Extensions.Add(extension); pipelineBuilder.Extensions.Add(softlineBreak); diff --git a/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml b/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml index bed3409c99..87439e2d36 100644 --- a/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml +++ b/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml @@ -21,6 +21,19 @@ x:Name="OpenUriDialog" x:Uid="OpenUriDialog" PrimaryButtonStyle="{ThemeResource AccentButtonStyle}" - SecondaryButtonClick="OpenUriDialog_SecondaryButtonClick" /> + SecondaryButtonClick="OpenUriDialog_SecondaryButtonClick"> + + + + + diff --git a/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml.cs b/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml.cs index e6d313d791..5a2e9900d0 100644 --- a/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml.cs +++ b/src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml.cs @@ -34,6 +34,11 @@ namespace Peek.FilePreviewer.Controls private Color? _originalBackgroundColor; + /// + /// URI of the current source being previewed (for resource filtering) + /// + private Uri? _currentSourceUri; + public delegate void NavigationCompletedHandler(WebView2? sender, CoreWebView2NavigationCompletedEventArgs? args); public delegate void DOMContentLoadedHandler(CoreWebView2? sender, CoreWebView2DOMContentLoadedEventArgs? args); @@ -97,6 +102,7 @@ namespace Peek.FilePreviewer.Controls { this.InitializeComponent(); Environment.SetEnvironmentVariable("WEBVIEW2_USER_DATA_FOLDER", TempFolderPath.Path, EnvironmentVariableTarget.Process); + Environment.SetEnvironmentVariable("WEBVIEW2_ADDITIONAL_BROWSER_ARGUMENTS", "--block-new-web-contents", EnvironmentVariableTarget.Process); } public void Dispose() @@ -105,6 +111,7 @@ namespace Peek.FilePreviewer.Controls { PreviewBrowser.CoreWebView2.DOMContentLoaded -= CoreWebView2_DOMContentLoaded; PreviewBrowser.CoreWebView2.ContextMenuRequested -= CoreWebView2_ContextMenuRequested; + RemoveResourceFilter(); } } @@ -123,6 +130,14 @@ namespace Peek.FilePreviewer.Controls { /* CoreWebView2.Navigate() will always trigger a navigation even if the content/URI is the same. * Use WebView2.Source to avoid re-navigating to the same content. */ + _currentSourceUri = Source; + + // Only apply resource filter for non-dev files + if (!IsDevFilePreview) + { + ApplyResourceFilter(); + } + PreviewBrowser.CoreWebView2.Navigate(Source.ToString()); } } @@ -146,10 +161,14 @@ namespace Peek.FilePreviewer.Controls if (IsDevFilePreview) { PreviewBrowser.CoreWebView2.SetVirtualHostNameToFolderMapping(Microsoft.PowerToys.FilePreviewCommon.MonacoHelper.VirtualHostName, Microsoft.PowerToys.FilePreviewCommon.MonacoHelper.MonacoDirectory, CoreWebView2HostResourceAccessKind.Allow); + + // Remove resource filter for dev files (Monaco needs to load resources) + RemoveResourceFilter(); } else { PreviewBrowser.CoreWebView2.ClearVirtualHostNameToFolderMapping(Microsoft.PowerToys.FilePreviewCommon.MonacoHelper.VirtualHostName); + ApplyResourceFilter(); } } } @@ -283,6 +302,66 @@ namespace Peek.FilePreviewer.Controls } } + /// + /// Applies strict resource filtering for non-dev files to block external resources. + /// This prevents XSS attacks and unwanted external content loading. + /// + private void ApplyResourceFilter() + { + // Remove existing handler to prevent duplicate subscriptions + RemoveResourceFilter(); + + // Add filter and subscribe to resource requests + PreviewBrowser.CoreWebView2.AddWebResourceRequestedFilter("*", CoreWebView2WebResourceContext.All); + PreviewBrowser.CoreWebView2.WebResourceRequested += CoreWebView2_WebResourceRequested; + } + + private void RemoveResourceFilter() + { + PreviewBrowser.CoreWebView2.WebResourceRequested -= CoreWebView2_WebResourceRequested; + } + + private void CoreWebView2_WebResourceRequested(CoreWebView2 sender, CoreWebView2WebResourceRequestedEventArgs args) + { + if (_currentSourceUri == null) + { + return; + } + + var requestUri = new Uri(args.Request.Uri); + + // Allow loading the source file itself + if (requestUri == _currentSourceUri) + { + return; + } + + // For local file:// resources, allow same directory and subdirectories + if (requestUri.Scheme == "file" && _currentSourceUri.Scheme == "file") + { + try + { + var sourceDirectory = System.IO.Path.GetDirectoryName(_currentSourceUri.LocalPath); + var requestPath = requestUri.LocalPath; + + // Allow resources in the same directory or subdirectories + if (!string.IsNullOrEmpty(sourceDirectory) && + requestPath.StartsWith(sourceDirectory, StringComparison.OrdinalIgnoreCase)) + { + return; + } + } + catch + { + // If path processing fails, block for security + } + } + + // Block all other resources including http(s) requests to prevent external tracking, + // data exfiltration, and XSS attacks + args.Response = PreviewBrowser.CoreWebView2.Environment.CreateWebResourceResponse(null, 403, "Forbidden", null); + } + private void CoreWebView2_DOMContentLoaded(CoreWebView2 sender, CoreWebView2DOMContentLoadedEventArgs args) { // If the file being previewed is HTML or HTM, reset the background color to its original state. @@ -344,7 +423,7 @@ namespace Peek.FilePreviewer.Controls private async Task ShowOpenUriDialogAsync(Uri uri) { - OpenUriDialog.Content = uri.ToString(); + OpenUriDialogContent.Text = uri.ToString(); var result = await OpenUriDialog.ShowAsync(); if (result == ContentDialogResult.Primary) @@ -356,7 +435,7 @@ namespace Peek.FilePreviewer.Controls private void OpenUriDialog_SecondaryButtonClick(ContentDialog sender, ContentDialogButtonClickEventArgs args) { var dataPackage = new DataPackage(); - dataPackage.SetText(sender.Content.ToString()); + dataPackage.SetText(OpenUriDialogContent.Text); Clipboard.SetContent(dataPackage); } } diff --git a/src/modules/peek/Peek.FilePreviewer/Previewers/WebBrowserPreviewer/WebBrowserPreviewer.cs b/src/modules/peek/Peek.FilePreviewer/Previewers/WebBrowserPreviewer/WebBrowserPreviewer.cs index bd9fb24cb3..4516b7fc29 100644 --- a/src/modules/peek/Peek.FilePreviewer/Previewers/WebBrowserPreviewer/WebBrowserPreviewer.cs +++ b/src/modules/peek/Peek.FilePreviewer/Previewers/WebBrowserPreviewer/WebBrowserPreviewer.cs @@ -113,38 +113,41 @@ namespace Peek.FilePreviewer.Previewers await Dispatcher.RunOnUiThread(async () => { - bool isHtml = File.Extension == ".html" || File.Extension == ".htm"; - bool isMarkdown = File.Extension == ".md"; - bool isSvg = File.Extension == ".svg"; + string extension = File.Extension; - bool supportedByMonaco = MonacoHelper.SupportedMonacoFileTypes.Contains(File.Extension); - bool useMonaco = supportedByMonaco && !isHtml && !isMarkdown && !isSvg; + // Default: non-dev file preview with standard context menu + IsDevFilePreview = false; + CustomContextMenu = false; - IsDevFilePreview = supportedByMonaco; - CustomContextMenu = useMonaco; - - if (useMonaco) + // Determine preview strategy based on file type priority + if (extension == ".md") { - var raw = await ReadHelper.Read(File.Path.ToString()); - Preview = new Uri(MonacoHelper.PreviewTempFile(raw, File.Extension, TempFolderPath.Path, _previewSettings.SourceCodeTryFormat, _previewSettings.SourceCodeWrapText, _previewSettings.SourceCodeStickyScroll, _previewSettings.SourceCodeFontSize, _previewSettings.SourceCodeMinimap)); - } - else if (isMarkdown) - { - IsDevFilePreview = false; + // Markdown files use custom renderer var raw = await ReadHelper.Read(File.Path.ToString()); Preview = new Uri(MarkdownHelper.PreviewTempFile(raw, File.Path, TempFolderPath.Path)); } - else if (isSvg) + else if (extension == ".svg") { // SVG files are rendered directly by WebView2 for better compatibility // with complex SVGs from Adobe Illustrator, Inkscape, etc. - IsDevFilePreview = false; Preview = new Uri(File.Path); } + else if (extension == ".html" || extension == ".htm") + { + // Simple html file to preview. Shouldn't do things like enabling scripts or using a virtual mapped directory. + Preview = new Uri(File.Path); + } + else if (MonacoHelper.SupportedMonacoFileTypes.Contains(extension)) + { + // Source code files use Monaco editor + IsDevFilePreview = true; + CustomContextMenu = true; + var raw = await ReadHelper.Read(File.Path.ToString()); + Preview = new Uri(MonacoHelper.PreviewTempFile(raw, extension, TempFolderPath.Path, _previewSettings.SourceCodeTryFormat, _previewSettings.SourceCodeWrapText, _previewSettings.SourceCodeStickyScroll, _previewSettings.SourceCodeFontSize, _previewSettings.SourceCodeMinimap)); + } else { - // Simple html file to preview. Shouldn't do things like enabling scripts or using a virtual mapped directory. - IsDevFilePreview = false; + // Fallback for other supported file types (e.g., PDF) Preview = new Uri(File.Path); } }); diff --git a/src/modules/peek/Peek.UI/Strings/en-us/Resources.resw b/src/modules/peek/Peek.UI/Strings/en-us/Resources.resw index f45f961268..26482bb75a 100644 --- a/src/modules/peek/Peek.UI/Strings/en-us/Resources.resw +++ b/src/modules/peek/Peek.UI/Strings/en-us/Resources.resw @@ -250,9 +250,17 @@ Dialog showed when an URI is clicked. Button to copy the URI. - Do you want Peek to open the external application? + Do you want Peek to open this link? Title of the dialog showed when an URI is clicked,"Peek" is the name of the utility. + + Security Warning + Title of the warning banner in the open URI dialog. + + + This link will open externally. Make sure you trust the source before proceeding. + Warning message displayed in the banner when opening an external URI. + ({1:N0} bytes) Displays total number of bytes. Don't localize the "{1:N0}" part.