Skip to content

Commit

Permalink
Allow wsl$ in file URIs; generally allow all URI schemes (#14993)
Browse files Browse the repository at this point in the history
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: #7526 (comment)
> 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 😄 while we work on features that'll make it even more broadly applicable.

Closes #10188
Closes #7562

(cherry picked from commit 65640f6)
Service-Card-Id: 88719284
Service-Version: 1.17
  • Loading branch information
zadjii-msft authored and DHowett committed Mar 31, 2023
1 parent 9670230 commit 31aa23f
Showing 1 changed file with 14 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 31aa23f

Please sign in to comment.