New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 759111 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
(inactive)
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Security



Sign in to add a comment

Security: Rendertron bugs

Reported by gra...@gmail.com, Aug 25 2017

Issue description

VULNERABILITY DETAILS
LFI & XSS bugs exist in a publicly shared open-source tool on GitHub called Rendertron -
 https://github.com/GoogleChrome/rendertron.

VERSION
Chrome Version: 60.0.3112.90 stable
Operating System: macOS Siera

REPRODUCTION CASE
1. Clone https://github.com/GoogleChrome/rendertron, npm install && npm start
2. LFI 1: http://localhost:3000/screenshot/file:///etc/passwd
   LFI 2: http://localhost:3000/render/file:///etc/passwd
3. XSS 1: http://localhost:3000/screenshot/<script>alert(document.domain)</script>
   XSS 2: http://localhost:3000/render/<script>alert(document.domain)</script>

Attached is a small shell script to reproduce the issues listed, it should be run inside the Rendertron project.
 
bugs.sh
436 bytes View Download
Cc: samli@google.com

Comment 2 by ta...@google.com, Aug 25 2017

Cc: -samli@google.com
Components: Internals>Headless
Labels: Security_Impact-Stable Security_Severity-Low OS-All
Owner: samli@chromium.org
Status: Assigned (was: Unconfirmed)
Assigning it to samli@ (as an owner of rendertron)

Comment 3 by ta...@google.com, Aug 25 2017

Cc: samli@google.com

Comment 4 by gra...@gmail.com, Aug 25 2017

In case it helps, these are some of the affected lines of the source:

LFI: 
https://github.com/GoogleChrome/rendertron/blob/master/src/main.js#L110
 https://github.com/GoogleChrome/rendertron/blob/master/src/main.js#L118
  https://github.com/GoogleChrome/rendertron/blob/master/src/renderer.js#L217-L221

XSS:
https://github.com/GoogleChrome/rendertron/blob/master/src/main.js#L105-L106
https://github.com/GoogleChrome/rendertron/blob/master/src/main.js#L129-L130

Also some possible fixes:
- Errors could be set to a more generic string (remove any variables / template strings)
- Filtering remote URL's on request may help

----------

Just caught a couple more issues, you may want to remove node_modules from being exposed, this can leak path information in installed package.json files.

Example: http://localhost:3000/node_modules/@google-cloud/common/package.json

Also noticed the route '/_ah/stop' is exposed allowing any user to stop the application.  Not sure if I would call this a DoS, but it effectively stops the service, and is available to any user.

Comment 5 by samli@chromium.org, Aug 25 2017

Thanks for reporting these, I'll take a look as soon as I can.
Project Member

Comment 6 by sheriffbot@chromium.org, Aug 26 2017

Labels: Pri-2

Comment 7 by gra...@gmail.com, Aug 29 2017

If it's allowed, I'd be happy to contribute a PR towards some of these fixes!

Comment 8 by samli@chromium.org, Aug 30 2017

PRs definitely welcome. I'll also be taking a look soon.

Comment 9 by gra...@gmail.com, Aug 30 2017

So I started on a PR for this, the description of the PR does not include any security information yet as this ticket view is not public yet.  Tried to keep the PR as general as possible, additionally contributing some style changes.

This is the PR on GitHub I've contributed to - https://github.com/GoogleChrome/rendertron/pull/88

The PR I've submitted covers the security checks I've tested for:
- 2 LFI on /screenshot & /render
- 2 XSS on /screenshot & /render
- DoS from /_ah/stop (stop now only works in development)
- Information Leak from /node_modules directory being overly exposed

It's important to note the tests will need to be changed for this.  They're currently failing locally and in CI because they rely on 'file://' for statusCode and shadow DOM tests.  The next step would be to setup a small mock server to test these against, this could be setup with a module such as nock.

P.S.:
It looks like someone else had also caught the XSS bug and submitted a fix, but this is not related or exposed from any work I've done so far - https://github.com/GoogleChrome/rendertron/pull/87

Hope this helps!

Comment 10 by gra...@gmail.com, Aug 30 2017

Fixed the tests to work in CI using the existing mock server as well as any small linting issues.

Comment 11 by samli@google.com, Aug 30 2017

Thanks for the PR. I've added some comments with just some nits to fix up. #87 is redundant after your PR, correct?

Comment 12 by samli@chromium.org, Aug 30 2017

Also unrelated, but do you have any opinion on this PR?
 https://github.com/GoogleChrome/rendertron/pull/80 

Comment 13 by gra...@gmail.com, Aug 30 2017

Thanks for the quick review on the PR, I'll push an update soon with those changes!

For #80, I'd probably disable it by default but expose the option to the developer or user with a warning. 

Comment 14 by gra...@gmail.com, Aug 30 2017

Also #87 would be redundant.

Comment 15 by samli@chromium.org, Aug 30 2017

Status: Verified (was: Assigned)
PR has been merged and deployed. Verified per repro steps in #1.
Project Member

Comment 16 by sheriffbot@chromium.org, Aug 31 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-NA
Project Member

Comment 18 by sheriffbot@chromium.org, Dec 7 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment