From 31aa23f7170d7ed3f8e7fe719d720c3d0c51d834 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Mar 2023 13:29:42 -0500 Subject: [PATCH] Allow wsl$ in file URIs; generally allow all URI schemes (#14993) Does two things related to URLs emitted via OSC8. * Allows `wsl$` and `wsl.localhost` as the hostname in `file://` URIs * Generally allows _all_ URIs that parse as a URI. The relevant security comments: https://github.com/microsoft/terminal/pull/7526#issuecomment-764160208 > this doesn't let a would-be attacker specify command-line arguments (ala "cmd.exe /s /c do_a_bad_thing") (using somebody else's reputation to cause mayhem) > > `ShellExecute` de-elevates because it bounces a launch request off the shell > > "Works predictably for 15% of applications" (h/t to PhMajerus' AXSH, and other on-Windows requestors) is better in so many ways than "Works for 0% of applications", in my estimation. Incremental progress :smile: while we work on features that'll make it even more broadly applicable. Closes #10188 Closes #7562 (cherry picked from commit 65640f6fe32ed8ff5a5a2d589f6230f453116f4d) Service-Card-Id: 88719284 Service-Version: 1.17 --- src/cascadia/TerminalApp/TerminalPage.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 94e9865e63a..f20c106ee0b 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -2473,14 +2473,27 @@ namespace winrt::TerminalApp::implementation { return true; } + + // GH#10188: WSL paths are okay. We'll let those through. + if (host == L"wsl$" || host == L"wsl.localhost") + { + return true; + } + // TODO: by the OSC 8 spec, if a hostname (other than localhost) is provided, we _should_ be // comparing that value against what is returned by GetComputerNameExW and making sure they match. // However, ShellExecute does not seem to be happy with file URIs of the form // file://{hostname}/path/to/file.ext // and so while we could do the hostname matching, we do not know how to actually open the URI // if its given in that form. So for now we ignore all hostnames other than localhost + return false; } - return false; + + // In this case, the app manually output a URI other than file:// or + // http(s)://. We'll trust the user knows what they're doing when + // clicking on those sorts of links. + // See discussion in GH#7562 for more details. + return true; } // Important! Don't take this eventArgs by reference, we need to extend the