diff --git a/CHANGELOG.md b/CHANGELOG.md index 3071b09ad..0bd63e496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,12 +9,22 @@ Forge ChangeLog maintainers for internal project debug purposes and was never intended to be used with untrusted user inputs. This API was not documented or advertised and is being removed rather than fixed. +- **SECURITY**, **BREAKING**: Remove `forge.util.parseUrl()` (and + `forge.http.parseUrl` alias) and use the [WHATWG URL + Standard](https://url.spec.whatwg.org/). `URL` is supported by modern browers + and modern Node.js. This change is needed to address URL parsing security + issues. If `forge.util.parseUrl()` is used directly or through `forge.xhr` or + `forge.http` APIs, and support is needed for environments without `URL` + support, then a polyfill must be used. - **BREAKING**: Remove `forge.task` API. This API was never used, documented, or advertised by the maintainers. If anyone was using this API and wishes to continue development it in other project, please let the maintainers know. Due to use in the test suite, a modified version is located in `tests/support/`. +### Changed +- **BREAKING**: Increase supported Node.js version to 6.13.0 for URL support. + ### Added - OIDs for `surname`, `title`, and `givenName`. @@ -23,6 +33,12 @@ Forge ChangeLog Depending on how applications used this id to name association it could cause compatibility issues. +### Notes +- The URL related changes may expose bugs in some of the networking related + code (unrelated to the much wider used cryptography code). The automated and + manual test coverage for this code is weak at best. Issues or patches to + update the code or tests would be appriciated. + ## 0.10.0 - 2020-09-01 ### Changed diff --git a/README.md b/README.md index c308ebe06..bddcffe6e 100644 --- a/README.md +++ b/README.md @@ -1968,10 +1968,6 @@ var nodeBuffer = Buffer.from(forgeBuffer.getBytes(), 'binary'); // make sure you specify the encoding as 'binary' var nodeBuffer = Buffer.from('CAFE', 'hex'); var forgeBuffer = forge.util.createBuffer(nodeBuffer.toString('binary')); - -// parse a URL -var parsed = forge.util.parseUrl('http://example.com/foo?bar=baz'); -// parsed.scheme, parsed.host, parsed.port, parsed.path, parsed.fullHost ``` diff --git a/lib/http.js b/lib/http.js index 0ae863050..fe52986b1 100644 --- a/lib/http.js +++ b/lib/http.js @@ -33,8 +33,8 @@ var _getStorageId = function(client) { // browsers (if this is undesirable) // navigator.userAgent return 'forge.http.' + - client.url.scheme + '.' + - client.url.host + '.' + + client.url.protocol.slice(0, -1) + '.' + + client.url.hostname + '.' + client.url.port; }; @@ -121,7 +121,7 @@ var _doRequest = function(client, socket) { // connect socket.options.request.connectTime = +new Date(); socket.connect({ - host: client.url.host, + host: client.url.hostname, port: client.url.port, policyPort: client.policyPort, policyUrl: client.policyUrl @@ -310,7 +310,7 @@ var _initSocket = function(client, socket, tlsOptions) { // prime socket by connecting and caching TLS session, will do // next request from there socket.connect({ - host: client.url.host, + host: client.url.hostname, port: client.url.port, policyPort: client.policyPort, policyUrl: client.policyUrl @@ -405,7 +405,7 @@ var _readCookies = function(client, response) { * * @param options: * url: the url to connect to (scheme://host:port). - * socketPool: the flash socket pool to use. + * socketPool: the flash socket pool to use. * policyPort: the flash policy port to use (if other than the * socket pool default), use 0 for flash default. * policyUrl: the flash policy file URL to use (if provided will @@ -441,8 +441,10 @@ http.createClient = function(options) { // get scheme, host, and port from url options.url = (options.url || window.location.protocol + '//' + window.location.host); - var url = http.parseUrl(options.url); - if(!url) { + var url; + try { + url = new URL(options.url); + } catch(e) { var error = new Error('Invalid url.'); error.details = {url: options.url}; throw error; @@ -469,7 +471,7 @@ http.createClient = function(options) { // idle sockets idle: [], // whether or not the connections are secure - secure: (url.scheme === 'https'), + secure: (url.protocol === 'https:'), // cookie jar (key'd off of name and then path, there is only 1 domain // and one setting for secure per client so name+path is unique) cookies: {}, @@ -497,7 +499,7 @@ http.createClient = function(options) { if(depth === 0 && verified === true) { // compare common name to url host var cn = certs[depth].subject.getField('CN'); - if(cn === null || client.url.host !== cn.value) { + if(cn === null || client.url.hostname !== cn.value) { verified = { message: 'Certificate common name does not match url host.' }; @@ -512,7 +514,7 @@ http.createClient = function(options) { tlsOptions = { caStore: caStore, cipherSuites: options.cipherSuites || null, - virtualHost: options.virtualHost || url.host, + virtualHost: options.virtualHost || url.hostname, verify: options.verify || _defaultCertificateVerify, getCertificate: options.getCertificate || null, getPrivateKey: options.getPrivateKey || null, @@ -552,7 +554,7 @@ http.createClient = function(options) { client.send = function(options) { // add host header if not set if(options.request.getField('Host') === null) { - options.request.setField('Host', client.url.fullHost); + options.request.setField('Host', client.url.origin); } // set default dummy handlers @@ -1307,15 +1309,6 @@ http.createResponse = function() { return response; }; -/** - * Parses the scheme, host, and port from an http(s) url. - * - * @param str the url string. - * - * @return the parsed url object or null if the url is invalid. - */ -http.parseUrl = forge.util.parseUrl; - /** * Returns true if the given url is within the given cookie's domain. * @@ -1336,11 +1329,11 @@ http.withinCookieDomain = function(url, cookie) { // ensure domain starts with a '.' // parse URL as necessary if(typeof url === 'string') { - url = http.parseUrl(url); + url = new URL(url); } - // add '.' to front of URL host to match against domain - var host = '.' + url.host; + // add '.' to front of URL hostname to match against domain + var host = '.' + url.hostname; // if the host ends with domain then it falls within it var idx = host.lastIndexOf(domain); diff --git a/lib/util.js b/lib/util.js index 98dfd3427..5100eab6e 100644 --- a/lib/util.js +++ b/lib/util.js @@ -2258,43 +2258,6 @@ util.clearItems = function(api, id, location) { _callStorageFunction(_clearItems, arguments, location); }; -/** - * Parses the scheme, host, and port from an http(s) url. - * - * @param str the url string. - * - * @return the parsed url object or null if the url is invalid. - */ -util.parseUrl = function(str) { - // FIXME: this regex looks a bit broken - var regex = /^(https?):\/\/([^:&^\/]*):?(\d*)(.*)$/g; - regex.lastIndex = 0; - var m = regex.exec(str); - var url = (m === null) ? null : { - full: str, - scheme: m[1], - host: m[2], - port: m[3], - path: m[4] - }; - if(url) { - url.fullHost = url.host; - if(url.port) { - if(url.port !== 80 && url.scheme === 'http') { - url.fullHost += ':' + url.port; - } else if(url.port !== 443 && url.scheme === 'https') { - url.fullHost += ':' + url.port; - } - } else if(url.scheme === 'http') { - url.port = 80; - } else if(url.scheme === 'https') { - url.port = 443; - } - url.full = url.scheme + '://' + url.fullHost; - } - return url; -}; - /* Storage for query variables */ var _queryVariables = null; diff --git a/lib/xhr.js b/lib/xhr.js index e493c3b60..fa928352b 100644 --- a/lib/xhr.js +++ b/lib/xhr.js @@ -151,7 +151,7 @@ xhrApi.init = function(options) { getPrivateKey: options.getPrivateKey, getSignature: options.getSignature }); - _clients[_client.url.full] = _client; + _clients[_client.url.origin] = _client; forge.log.debug(cat, 'ready'); }; @@ -380,8 +380,10 @@ xhrApi.create = function(options) { // use default _state.client = _client; } else { - var url = http.parseUrl(options.url); - if(!url) { + var url; + try { + url = new URL(options.url); + } catch(e) { var error = new Error('Invalid url.'); error.details = { url: options.url @@ -389,9 +391,9 @@ xhrApi.create = function(options) { } // find client - if(url.full in _clients) { + if(url.origin in _clients) { // client found - _state.client = _clients[url.full]; + _state.client = _clients[url.origin]; } else { // create client _state.client = http.createClient({ @@ -409,7 +411,7 @@ xhrApi.create = function(options) { getPrivateKey: options.getPrivateKey, getSignature: options.getSignature }); - _clients[url.full] = _state.client; + _clients[url.origin] = _state.client; } } diff --git a/package.json b/package.json index f54ff72d3..22ff40e46 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "dist/*.min.js.map" ], "engines": { - "node": ">= 6.0.0" + "node": ">= 6.13.0" }, "keywords": [ "aes", diff --git a/tests/websockets/server-webid.js b/tests/websockets/server-webid.js index 6f7cf37b8..5319372bb 100644 --- a/tests/websockets/server-webid.js +++ b/tests/websockets/server-webid.js @@ -174,9 +174,10 @@ var fetchUrl = function(url, callback, redirects) { console.log('Fetching URL: \"' + url + '\"'); // parse URL - url = forge.util.parseUrl(url); - var client = http.createClient( - url.port, url.fullHost, url.scheme === 'https'); + url = new URL(url); + var client = http.createClient({ + url: url + }); var request = client.request('GET', url.path, { Host: url.host, Accept: 'application/rdf+xml'