diff options
| -rw-r--r-- | app/assets/javascripts/lib/dompurify.js | 19 | ||||
| -rw-r--r-- | app/assets/javascripts/lib/utils/url_utility.js | 18 | ||||
| -rw-r--r-- | spec/frontend/lib/dompurify_spec.js | 6 | ||||
| -rw-r--r-- | spec/frontend/lib/utils/url_utility_spec.js | 21 |
4 files changed, 55 insertions, 9 deletions
diff --git a/app/assets/javascripts/lib/dompurify.js b/app/assets/javascripts/lib/dompurify.js index d421d66981e..47ede8cb1bb 100644 --- a/app/assets/javascripts/lib/dompurify.js +++ b/app/assets/javascripts/lib/dompurify.js @@ -1,5 +1,5 @@ import { sanitize as dompurifySanitize, addHook } from 'dompurify'; -import { getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility'; +import { getNormalizedURL, getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility'; const defaultConfig = { // Safely allow SVG <use> tags @@ -11,12 +11,14 @@ const defaultConfig = { // Only icons urls from `gon` are allowed const getAllowedIconUrls = (gon = window.gon) => - [gon.sprite_file_icons, gon.sprite_icons].filter(Boolean); + [gon.sprite_file_icons, gon.sprite_icons] + .filter(Boolean) + .map((path) => relativePathToAbsolute(path, getBaseURL())); -const isUrlAllowed = (url) => getAllowedIconUrls().some((allowedUrl) => url.startsWith(allowedUrl)); +const isUrlAllowed = (url) => + getAllowedIconUrls().some((allowedUrl) => getNormalizedURL(url).startsWith(allowedUrl)); -const isHrefSafe = (url) => - isUrlAllowed(url) || isUrlAllowed(relativePathToAbsolute(url, getBaseURL())) || url.match(/^#/); +const isHrefSafe = (url) => url.match(/^#/) || isUrlAllowed(url); const removeUnsafeHref = (node, attr) => { if (!node.hasAttribute(attr)) { @@ -36,13 +38,14 @@ const removeUnsafeHref = (node, attr) => { * <use href="/assets/icons-xxx.svg#icon_name"></use> * </svg> * + * It validates both href & xlink:href attributes. + * Note that `xlink:href` is deprecated, but still in use + * https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href + * * @param {Object} node - Node to sanitize */ const sanitizeSvgIcon = (node) => { removeUnsafeHref(node, 'href'); - - // Note: `xlink:href` is deprecated, but still in use - // https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href removeUnsafeHref(node, 'xlink:href'); }; diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 1c22d21a313..c70d23d06ec 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -399,6 +399,24 @@ export function isSafeURL(url) { } } +/** + * Returns a normalized url + * + * https://gitlab.com/foo/../baz => https://gitlab.com/baz + * + * @param {String} url - URL to be transformed + * @param {String?} baseUrl - current base URL + * @returns {String} + */ +export const getNormalizedURL = (url, baseUrl) => { + const base = baseUrl || getBaseURL(); + try { + return new URL(url, base).href; + } catch (e) { + return ''; + } +}; + export function getWebSocketProtocol() { return window.location.protocol.replace('http', 'ws'); } diff --git a/spec/frontend/lib/dompurify_spec.js b/spec/frontend/lib/dompurify_spec.js index 324441fa2c9..47a94a4dcde 100644 --- a/spec/frontend/lib/dompurify_spec.js +++ b/spec/frontend/lib/dompurify_spec.js @@ -22,12 +22,16 @@ const safeUrls = { const unsafeUrls = [ '/an/evil/url', '../../../evil/url', - 'https://evil.url/assets/icons-123a.svg', + 'https://evil.url/assets/icons-123a.svg#test', 'https://evil.url/assets/icons-456b.svg', `https://evil.url/${rootGon.sprite_icons}`, `https://evil.url/${rootGon.sprite_file_icons}`, `https://evil.url/${absoluteGon.sprite_icons}`, `https://evil.url/${absoluteGon.sprite_file_icons}`, + `${rootGon.sprite_icons}/../evil/path`, + `${rootGon.sprite_file_icons}/../../evil/path`, + `${absoluteGon.sprite_icons}/../evil/path`, + `${absoluteGon.sprite_file_icons}/../../https://evil.url`, ]; const forbiddenDataAttrs = ['data-remote', 'data-url', 'data-type', 'data-method']; diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js index 18b68d91e01..36e1a453ef4 100644 --- a/spec/frontend/lib/utils/url_utility_spec.js +++ b/spec/frontend/lib/utils/url_utility_spec.js @@ -607,6 +607,27 @@ describe('URL utility', () => { }); }); + describe('getNormalizedURL', () => { + it.each` + url | base | result + ${'./foo'} | ${''} | ${'http://test.host/foo'} + ${'../john.md'} | ${''} | ${'http://test.host/john.md'} + ${'/images/img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/images/img.png'} + ${'/images/../img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/img.png'} + ${'/images/./img.png'} | ${'https://gitlab.com'} | ${'https://gitlab.com/images/img.png'} + ${'./images/img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/user/images/img.png'} + ${'../images/../img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'} + ${'/images/img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'} + ${'/images/../img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/img.png'} + ${'/images/./img.png'} | ${'https://gitlab.com/user/project'} | ${'https://gitlab.com/images/img.png'} + `( + 'converts url "$url" with base "$base" to normalized url => "expected"', + ({ url, base, result }) => { + expect(urlUtils.getNormalizedURL(url, base)).toBe(result); + }, + ); + }); + describe('getWebSocketProtocol', () => { it.each` protocol | expectation |