diff --git a/api/utils/render.js b/api/utils/render.js index 736ef64d666..429ec2b3b5b 100644 --- a/api/utils/render.js +++ b/api/utils/render.js @@ -28,6 +28,54 @@ var countlyConfig = require('./../config', 'dont-enclose'); var fs = require('fs'); +/** +* Check that a view stays on the dashboard, and return the url to navigate to +* +* The view can come from a request (/o/render passes params.qstring.view through), and the +* url is built by concatenation. With the default countlyConfig.path of "" the prefix is +* exactly "http://localhost", so a view that does not begin with "/" rewrites the host +* instead of the path: +* +* "@169.254.169.254/latest/meta-data/" -> host 169.254.169.254 +* ":8500/v1/kv/?recurse" -> host localhost:8500 +* ".internal.example/x" -> host localhost.internal.example +* +* Parsing the concatenated url and comparing origins settles all of those at once, rather +* than trying to enumerate them. It also agrees with what Chromium will do with the same +* string, since both use the WHATWG url parser. Note that a private range denylist would +* be the wrong control here: the intended target is loopback. +* +* The returned url is the concatenation itself, unchanged, so a configured +* countlyConfig.path keeps working exactly as before. +* @param {string} host - dashboard origin plus the configured path +* @param {string} view - view to render, expected to be a path on that dashboard +* @returns {string|null} url to navigate to, or null when it leaves the dashboard origin +**/ +function sameOriginView(host, view) { + if (typeof view !== "string") { + return null; + } + var target = host + view; + var expected; + var actual; + try { + //host may carry no path at all, so normalise it before taking the origin + expected = new URL(host + "/").origin; + actual = new URL(target).origin; + } + catch (error) { + return null; + } + //opaque origins serialise to "null" and would compare equal to each other + if (!expected || expected === "null" || actual !== expected) { + return null; + } + return target; +} + +//exported so the same origin check can be unit tested without launching a browser +exports.sameOriginView = sameOriginView; + /** * Function to render views as images * @param {object} options - options required for rendering @@ -128,6 +176,38 @@ exports.renderView = function(options, cb) { scale: options.dimensions && options.dimensions.scale ? options.dimensions.scale : 2 }; + //Second, independent control: the renderer may only fetch from the + //dashboard origin. The check on the view bounds where we navigate, this + //bounds every subresource the rendered page then asks for. Same approach as + //api/utils/pdf.js. The dashboard serves all of its own assets, so nothing in + //a normal render is refused here. + var renderOrigin = null; + try { + renderOrigin = new URL(host + "/").origin; + } + catch (error) { + log.e("Cannot parse the configured dashboard host", host); + } + await page.setRequestInterception(true); + page.on('request', function(request) { + var requestUrl = request.url(); + if (/^(data|blob|about):/.test(requestUrl)) { + return request.continue(); + } + var requestOrigin; + try { + requestOrigin = new URL(requestUrl).origin; + } + catch (error) { + requestOrigin = null; + } + if (renderOrigin && renderOrigin !== "null" && requestOrigin === renderOrigin) { + return request.continue(); + } + log.d("Refused a request outside the dashboard origin", requestUrl); + return request.abort(); + }); + page.setDefaultNavigationTimeout(updatedTimeout); const resp = await page.goto(host + '/login/token/' + token + '?ssr=true'); const status = resp?.status(); @@ -139,7 +219,14 @@ exports.renderView = function(options, cb) { await timeout(1500); - await page.goto(host + view); + var viewUrl = sameOriginView(host, view); + if (!viewUrl) { + //the value can be attacker supplied, so keep it out of the log + log.e("Refusing to render a view outside the dashboard origin"); + throw new Error("Invalid view"); + } + + await page.goto(viewUrl); if (waitForRegex) { await page.waitForResponse( diff --git a/test/unit-tests/api.utils.render.js b/test/unit-tests/api.utils.render.js new file mode 100644 index 00000000000..0025040dc08 --- /dev/null +++ b/test/unit-tests/api.utils.render.js @@ -0,0 +1,88 @@ +var should = require("should"); +var render = require("../../api/utils/render"); + +// The renderer builds the url it opens by concatenating the dashboard host with the +// requested view, and /o/render lets the caller choose that view. A view that does not +// begin with "/" therefore changes the host rather than the path, which would point the +// headless browser at whatever the server itself can reach. sameOriginView is the check +// that keeps the navigation on the dashboard. +// +// These tests use hostnames and IP literals only, so nothing here resolves DNS or opens +// a browser. +describe("render same origin view check", function() { + var HOSTS = [ + "http://localhost", // the default, countlyConfig.path is "" out of the box + "http://localhost/countly", // a configured countlyConfig.path + "https://dash.example.com:8443" // https on a non default port + ]; + + describe("refuses views that move the navigation off the dashboard", function() { + // each of these rewrites the host when concatenated onto a host with no path + var offOrigin = [ + "@169.254.169.254/latest/meta-data/iam/security-credentials/", + "@[::ffff:169.254.169.254]/latest/meta-data/", + "@169.254.169.254:80/latest/", + "@localhost:9200/_cluster/health", + ":8500/v1/kv/?recurse", + ":6379/", + ".internal.example/x", + // the url parser strips tabs, newlines and carriage returns before it + // parses, so these reach the same host as the plain payload above + "\u0009@169.254.169.254/latest/", + "\u000a@169.254.169.254/latest/", + "\u000d@169.254.169.254/latest/" + ]; + offOrigin.forEach(function(view) { + it("refuses " + JSON.stringify(view), function() { + should.not.exist(render.sameOriginView("http://localhost", view)); + should.not.exist(render.sameOriginView("https://dash.example.com:8443", view)); + }); + }); + }); + + describe("keeps every view the dashboard itself renders", function() { + var allowed = [ + "/#/dashboard", + "/dashboard?ssr=true#/custom/6a41837e902bfd5369ddc610", // the email report screenshot + "/#/6a41837e902bfd5369ddc610/analytics/sessions", + "/#/manage/users", + "/", + "", + "?ssr=true", + "//assets/x" // a path on our own host, not a protocol relative url + ]; + HOSTS.forEach(function(host) { + allowed.forEach(function(view) { + it("allows " + JSON.stringify(view) + " on " + host, function() { + // and returns the concatenation unchanged, so a configured + // countlyConfig.path keeps working exactly as it did before + render.sameOriginView(host, view).should.equal(host + view); + }); + }); + }); + }); + + it("does not treat a configured path prefix as part of the host", function() { + // with a path prefix the same payloads land in the path, where they are harmless, + // and the url is still the one the renderer would have opened before + render.sameOriginView("http://localhost/countly", "@169.254.169.254/latest/") + .should.equal("http://localhost/countly@169.254.169.254/latest/"); + }); + + it("refuses a view that is not a string", function() { + should.not.exist(render.sameOriginView("http://localhost", undefined)); + should.not.exist(render.sameOriginView("http://localhost", null)); + should.not.exist(render.sameOriginView("http://localhost", {})); + should.not.exist(render.sameOriginView("http://localhost", 5)); + }); + + it("refuses everything when the configured host cannot be parsed", function() { + should.not.exist(render.sameOriginView("not a url", "/#/dashboard")); + should.not.exist(render.sameOriginView("", "/#/dashboard")); + }); + + it("refuses a host whose origin is opaque, so two opaque origins cannot match", function() { + // a protocol other than http(s) serialises its origin as "null" + should.not.exist(render.sameOriginView("file://localhost", "/#/dashboard")); + }); +});