From 8b1b427195d9f7fb9db8246a53f0dd68403b0a01 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Mon, 9 May 2016 09:57:07 +0200 Subject: [PATCH] feat(security): support transform CSS functions for sanitization. Fixes part of #8514. --- .../src/security/style_sanitizer.ts | 25 +++++++++++-------- .../test/security/style_sanitizer_spec.ts | 18 ++++++++----- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/modules/@angular/platform-browser/src/security/style_sanitizer.ts b/modules/@angular/platform-browser/src/security/style_sanitizer.ts index 78bcb94df9..6de0692670 100644 --- a/modules/@angular/platform-browser/src/security/style_sanitizer.ts +++ b/modules/@angular/platform-browser/src/security/style_sanitizer.ts @@ -4,19 +4,24 @@ import {assertionsEnabled} from '../../src/facade/lang'; /** * Regular expression for safe style values. * - * Quotes (" and ') are allowed, but a check must be done elsewhere to ensure - * they're balanced. + * Quotes (" and ') are allowed, but a check must be done elsewhere to ensure they're balanced. * - * ',' allows multiple values to be assigned to the same property - * (e.g. background-attachment or font-family) and hence could allow - * multiple values to get injected, but that should pose no risk of XSS. + * ',' allows multiple values to be assigned to the same property (e.g. background-attachment or + * font-family) and hence could allow multiple values to get injected, but that should pose no risk + * of XSS. * - * The rgb() and rgba() expression checks only for XSS safety, not for CSS - * validity. + * The function expression checks only for XSS safety, not for CSS validity. * - * This regular expression was taken from the Closure sanitization library. + * This regular expression was taken from the Closure sanitization library, and augmented for + * transformation values. */ -const SAFE_STYLE_VALUE = /^([-,."'%_!# a-zA-Z0-9]+|(?:rgb|hsl)a?\([0-9.%, ]+\))$/; +const VALUES = '[-,."\'%_!# a-zA-Z0-9]+'; +const TRANSFORMATION_FNS = '(?:matrix|translate|scale|rotate|skew|perspective)(?:X|Y|3d)?'; +const COLOR_FNS = '(?:rgb|hsl)a?'; +const FN_ARGS = '\\([-0-9.%, a-zA-Z]+\\)'; + +const SAFE_STYLE_VALUE = + new RegExp(`^(${VALUES}|(?:${TRANSFORMATION_FNS}|${COLOR_FNS})${FN_ARGS})$`, 'g'); /** * Checks that quotes (" and ') are properly balanced inside a string. Assumes @@ -45,7 +50,7 @@ function hasBalancedQuotes(value: string) { * value) and returns a value that is safe to use in a browser environment. */ export function sanitizeStyle(value: string): string { - value = String(value); // Make sure it's actually a string. + value = String(value).trim(); // Make sure it's actually a string. if (value.match(SAFE_STYLE_VALUE) && hasBalancedQuotes(value)) return value; if (assertionsEnabled()) { 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 e5c66b35b6..631889566b 100644 --- a/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts +++ b/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts @@ -15,14 +15,20 @@ export function main() { }); t.afterEach(() => { getDOM().log = originalLog; }); + function expectSanitize(v: string) { return t.expect(sanitizeStyle(v)); } 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/); + expectSanitize('abc').toEqual('abc'); + expectSanitize('50px').toEqual('50px'); + expectSanitize('rgb(255, 0, 0)').toEqual('rgb(255, 0, 0)'); + expectSanitize('expression(haha)').toEqual('unsafe'); + }); + t.it('rejects unblanaced quotes', () => { expectSanitize('"value" "').toEqual('unsafe'); }); + t.it('accepts transform functions', () => { + expectSanitize('rotate(90deg)').toEqual('rotate(90deg)'); + expectSanitize('rotate(javascript:evil())').toEqual('unsafe'); + expectSanitize('translateX(12px, -5px)').toEqual('translateX(12px, -5px)'); + expectSanitize('scale3d(1, 1, 2)').toEqual('scale3d(1, 1, 2)'); }); }); }