From 3e68b7eb1f35eefe4d95c007fae1500f468fd39b Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Mon, 9 May 2016 16:46:31 +0200 Subject: [PATCH] feat(security): warn users when sanitizing in dev mode. This should help developers to figure out what's going on when the sanitizer strips some input. Fixes #8522. --- .../src/security/html_sanitizer.ts | 2 +- .../src/security/style_sanitizer.ts | 8 ++++++++ .../src/security/url_sanitizer.ts | 10 +++++++++- .../test/security/html_sanitizer_spec.ts | 2 +- .../test/security/style_sanitizer_spec.ts | 15 +++++++++++++++ .../test/security/url_sanitizer_spec.ts | 18 ++++++++++++++++++ 6 files changed, 52 insertions(+), 3 deletions(-) diff --git a/modules/@angular/platform-browser/src/security/html_sanitizer.ts b/modules/@angular/platform-browser/src/security/html_sanitizer.ts index 74a8779739..0c423e6a9a 100644 --- a/modules/@angular/platform-browser/src/security/html_sanitizer.ts +++ b/modules/@angular/platform-browser/src/security/html_sanitizer.ts @@ -253,7 +253,7 @@ export function sanitizeHtml(unsafeHtml: string): string { } if (assertionsEnabled() && safeHtml !== unsafeHtml) { - DOM.log('WARNING: some HTML contents were removed during sanitization.'); + DOM.log('WARNING: sanitizing HTML stripped some content.'); } return safeHtml; diff --git a/modules/@angular/platform-browser/src/security/style_sanitizer.ts b/modules/@angular/platform-browser/src/security/style_sanitizer.ts index c0134d70af..78bcb94df9 100644 --- a/modules/@angular/platform-browser/src/security/style_sanitizer.ts +++ b/modules/@angular/platform-browser/src/security/style_sanitizer.ts @@ -1,3 +1,6 @@ +import {getDOM} from '../dom/dom_adapter'; +import {assertionsEnabled} from '../../src/facade/lang'; + /** * Regular expression for safe style values. * @@ -44,5 +47,10 @@ function hasBalancedQuotes(value: string) { export function sanitizeStyle(value: string): string { value = String(value); // Make sure it's actually a string. if (value.match(SAFE_STYLE_VALUE) && hasBalancedQuotes(value)) return value; + + if (assertionsEnabled()) { + getDOM().log('WARNING: sanitizing unsafe style value ' + value); + } + return 'unsafe'; } diff --git a/modules/@angular/platform-browser/src/security/url_sanitizer.ts b/modules/@angular/platform-browser/src/security/url_sanitizer.ts index 64b6779d13..7d496f9b3e 100644 --- a/modules/@angular/platform-browser/src/security/url_sanitizer.ts +++ b/modules/@angular/platform-browser/src/security/url_sanitizer.ts @@ -1,3 +1,6 @@ +import {getDOM} from '../dom/dom_adapter'; +import {assertionsEnabled} from '../../src/facade/lang'; + /** * A pattern that recognizes a commonly useful subset of URLs that are safe. * @@ -27,6 +30,11 @@ const SAFE_URL_PATTERN = /^(?:(?:https?|mailto|ftp|tel|file):|[^&:/?#]*(?:[/?#]|$))/gi; export function sanitizeUrl(url: string): string { - if (String(url).match(SAFE_URL_PATTERN)) return url; + url = String(url); + if (url.match(SAFE_URL_PATTERN)) return url; + + if (assertionsEnabled()) { + getDOM().log('WARNING: sanitizing unsafe URL value ' + url); + } return 'unsafe:' + url; } diff --git a/modules/@angular/platform-browser/test/security/html_sanitizer_spec.ts b/modules/@angular/platform-browser/test/security/html_sanitizer_spec.ts index 8dd0dbdb05..c38199fa14 100644 --- a/modules/@angular/platform-browser/test/security/html_sanitizer_spec.ts +++ b/modules/@angular/platform-browser/test/security/html_sanitizer_spec.ts @@ -39,7 +39,7 @@ export function main() { t.it('ignores non-element, non-attribute nodes', () => { t.expect(sanitizeHtml('no.')).toEqual('no.'); t.expect(sanitizeHtml('no.')).toEqual('no.'); - t.expect(logMsgs.join('\n')).toMatch(/HTML contents were removed during sanitization/); + t.expect(logMsgs.join('\n')).toMatch(/sanitizing HTML stripped some content/); }); t.it('escapes entities', () => { t.expect(sanitizeHtml('

Hello < World

')).toEqual('

Hello < World

'); diff --git a/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts b/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts index dedd346da1..e5c66b35b6 100644 --- a/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts +++ b/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts @@ -1,13 +1,28 @@ import * as t from '@angular/core/testing/testing_internal'; + +import {getDOM} from '../../src/dom/dom_adapter'; import {sanitizeStyle} from '../../src/security/style_sanitizer'; export function main() { t.describe('Style sanitizer', () => { + let logMsgs: string[]; + let originalLog: (msg: any) => any; + + t.beforeEach(() => { + logMsgs = []; + originalLog = getDOM().log; // Monkey patch DOM.log. + getDOM().log = (msg) => logMsgs.push(msg); + }); + t.afterEach(() => { getDOM().log = originalLog; }); + + t.it('sanitizes values', () => { t.expect(sanitizeStyle('abc')).toEqual('abc'); t.expect(sanitizeStyle('expression(haha)')).toEqual('unsafe'); // Unbalanced quotes. t.expect(sanitizeStyle('"value" "')).toEqual('unsafe'); + + t.expect(logMsgs.join('\n')).toMatch(/sanitizing unsafe style value/); }); }); } diff --git a/modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts b/modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts index 22da293932..7ae27d1f45 100644 --- a/modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts +++ b/modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts @@ -1,8 +1,26 @@ import * as t from '@angular/core/testing/testing_internal'; + +import {getDOM} from '../../src/dom/dom_adapter'; import {sanitizeUrl} from '../../src/security/url_sanitizer'; export function main() { t.describe('URL sanitizer', () => { + let logMsgs: string[]; + let originalLog: (msg: any) => any; + + t.beforeEach(() => { + logMsgs = []; + originalLog = getDOM().log; // Monkey patch DOM.log. + getDOM().log = (msg) => logMsgs.push(msg); + }); + t.afterEach(() => { getDOM().log = originalLog; }); + + t.it('reports unsafe URLs', () => { + t.expect(sanitizeUrl('javascript:evil()')).toBe('unsafe:javascript:evil()'); + t.expect(logMsgs.join('\n')).toMatch(/sanitizing unsafe URL value/); + }); + + t.describe('valid URLs', () => { const validUrls = [ '',