Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for viewing external sandboxed URLs in the Viewer pane #2252

Open
jmcphers opened this issue Feb 22, 2018 · 7 comments
Open

Add support for viewing external sandboxed URLs in the Viewer pane #2252

jmcphers opened this issue Feb 22, 2018 · 7 comments
Labels

Comments

@jmcphers
Copy link
Member

@jmcphers jmcphers commented Feb 22, 2018

In RStudio Desktop 1.1, we are very careful to disallow navigation to external URLs for security reasons. See e.g. here:

bool WebPage::acceptNavigationRequest(const QUrl &url, NavigationType navType, bool isMainFrame)

Now that we are on QtWebEngine, however, we have access to a working sandbox for IFrames; this has been part of Chromium for some time.

https://www.chromestatus.com/feature/5715536319086592

We could choose to enable external URLs in the Viewer, which would open up a number of possibilities for external service integration with the IDE, if we can validate that sandboxing them allows us to do so safely and securely.

@jmcphers
Copy link
Member Author

@jmcphers jmcphers commented Mar 30, 2018

Some investigation notes:

  1. I have confirmed that sandboxing works correctly on QtWebEngine as used by RStudio; a sample page has access to window.parent without the sandbox attribute, and does not have access with it.
  2. We are forced to pass the --no-sandbox flag to Chromium on Linux (see #2381). There was some speculation that this flag would make <iframe> sandboxing not work; however, some investigation revealed that iframe sandboxing does indeed still work even with --no-sandbox (it's not the same kind of sandboxing).
@yihui
Copy link
Member

@yihui yihui commented Apr 3, 2018

I just ran into this issue, and I'm glad that it has already been tracked here. Thanks!

yihui added a commit to yihui/servr that referenced this issue Apr 3, 2018
with RStudio 1.2.x, rstudioapi::viewer('https://www.rstudio.com') will open the URL in an external browser rstudio/rstudio#2252
@jmcphers jmcphers added this to the v1.2 milestone Apr 12, 2018
@jmcphers jmcphers closed this in 6bb5062 Apr 25, 2018
@jmcphers
Copy link
Member Author

@jmcphers jmcphers commented Apr 25, 2018

This is implemented now.

Note that sandboxing introduces a bunch of restrictions:

  • Only the requested URL (and URLs that have the same prefix) will load in the Viewer pane.
  • Any navigation attempts (including user-initiated navigation, script-initiated navigation, and even iframes) for other URLs will load outside RStudio.
  • We allow script execution (since almost everything relies on it these days) but disallow everything else that the spec allows. We can adjust these privileges as time goes on.
@jmcphers
Copy link
Member Author

@jmcphers jmcphers commented Apr 25, 2018

Also, for QA: the sandbox attribute is applied dynamically; it is added when browsing to a non-local URL, and removed when browsing to a local one.

@mine-cetinkaya-rundel
Copy link

@mine-cetinkaya-rundel mine-cetinkaya-rundel commented Sep 5, 2019

It looks like this is no longer working.

rstudioapi::viewer("http://google.com/")

and a few other URLs I tried don't actually show up in the viewer.

@ronblum ronblum reopened this Sep 5, 2019
@coatless
Copy link

@coatless coatless commented Oct 8, 2019

I can confirm that as of RStudio v1.2.1522

The only way around this is:

tfile = tempfile(fileext=".html")
download.file("http://google.com/", tfile)
rstudioapi::viewer(tfile)

Even then, clicking on a link will trigger a browser window to open...

coatless added a commit to r-assist/searcher that referenced this issue Oct 8, 2019
coatless added a commit to r-assist/searcher that referenced this issue Oct 8, 2019
#22)

* Enable opening the URL inside of the RStudio window.

* Update README file with new option

* Add a NOTE

* Switch default to `FALSE` as RStudio is not appropriately sandboxing the environment.

rstudio/rstudio#2252
@jmcphers jmcphers modified the milestones: v1.2, v1.2-preview Nov 1, 2019
@rundel
Copy link

@rundel rundel commented Dec 4, 2019

I have been messing around with this and it seems like this is being caused by the WebPage::acceptNavigationRequest method in DesktopWebPage.cpp. Specifically, there is currently a (very limited) list of safe hosts:

void initSafeHosts()
{
safeHosts_ = {
".youtube.com",
".vimeo.com",
".c9.ms"
};
for (const SessionServer& server : sessionServerSettings().servers())
{
http::URL url(server.url());
safeHosts_.push_back(url.hostname());
}
}
bool isSafeHost(const std::string& host)
{
boost::call_once(s_once,
initSafeHosts);
for (auto entry : safeHosts_)
if (boost::algorithm::ends_with(host, entry))
return true;
return false;
}

which are allowed to be opened by the viewer.

Is the safe hosts concept still necessary? If so is it possible to relax it somewhat such that users are warned about visiting an unsafe host and then able to progress or not via a modal dialog?

Also, while I was looking through the code it seemed like it would also be useful to expand the isLocal checks to include 0.0.0.0 as well as the machines ips.

@jmcphers jmcphers removed this from the v1.2 milestone Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.