From 28778e6c1b0fc9b3662a42d0883f7a3b9ab8c7d7 Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Fri, 12 Jul 2013 13:36:20 -0500 Subject: [PATCH] Rejects incorrect url in download manager --- src/annotation.js | 24 +++--------------------- src/util.js | 22 ++++++++++++++++++++++ web/download_manager.js | 6 +++++- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/annotation.js b/src/annotation.js index 3e747f039..b24ec5489 100644 --- a/src/annotation.js +++ b/src/annotation.js @@ -16,7 +16,7 @@ */ /* globals Util, isDict, isName, stringToPDFString, TODO, Dict, Stream, stringToBytes, PDFJS, isWorker, assert, NotImplementedException, - Promise, isArray, ObjectLoader */ + Promise, isArray, ObjectLoader, isValidUrl */ 'use strict'; @@ -641,24 +641,6 @@ var TextAnnotation = (function TextAnnotationClosure() { })(); var LinkAnnotation = (function LinkAnnotationClosure() { - function isValidUrl(url) { - if (!url) - return false; - var colon = url.indexOf(':'); - if (colon < 0) - return false; - var protocol = url.substr(0, colon); - switch (protocol) { - case 'http': - case 'https': - case 'ftp': - case 'mailto': - return true; - default: - return false; - } - } - function LinkAnnotation(params) { Annotation.call(this, params); @@ -676,7 +658,7 @@ var LinkAnnotation = (function LinkAnnotationClosure() { var url = action.get('URI'); // TODO: pdf spec mentions urls can be relative to a Base // entry in the dictionary. - if (!isValidUrl(url)) { + if (!isValidUrl(url, false)) { url = ''; } data.url = url; @@ -692,7 +674,7 @@ var LinkAnnotation = (function LinkAnnotationClosure() { // TODO: pdf reference says that GoToR // can also have 'NewWindow' attribute - if (!isValidUrl(url)) { + if (!isValidUrl(url, false)) { url = ''; } data.url = url; diff --git a/src/util.js b/src/util.js index 3f03fd62b..8cee4a1eb 100644 --- a/src/util.js +++ b/src/util.js @@ -108,6 +108,28 @@ function combineUrl(baseUrl, url) { } } +// Validates if URL is safe and allowed, e.g. to avoid XSS. +function isValidUrl(url, allowRelative) { + if (!url) { + return false; + } + var colon = url.indexOf(':'); + if (colon < 0) { + return allowRelative; + } + var protocol = url.substr(0, colon); + switch (protocol) { + case 'http': + case 'https': + case 'ftp': + case 'mailto': + return true; + default: + return false; + } +} +PDFJS.isValidUrl = isValidUrl; + // In a well-formed PDF, |cond| holds. If it doesn't, subsequent // behavior is undefined. function assertWellFormed(cond, msg) { diff --git a/web/download_manager.js b/web/download_manager.js index edf5da1af..eb455b862 100644 --- a/web/download_manager.js +++ b/web/download_manager.js @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/* globals URL*/ +/* globals URL, PDFJS */ 'use strict'; @@ -59,6 +59,10 @@ var DownloadManager = (function DownloadManagerClosure() { DownloadManager.prototype = { downloadUrl: function DownloadManager_downloadUrl(url, filename) { + if (!PDFJS.isValidUrl(url, true)) { + return; // restricted/invalid URL + } + download(url + '#pdfjs.action=download', filename); },