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)'); }); }); }