From 15ae710d22d1aa8f07d07745ed0bb6830dc28580 Mon Sep 17 00:00:00 2001
From: Martin Probst <martin@probst.io>
Date: Sun, 15 May 2016 11:33:47 +0200
Subject: [PATCH] feat(security): allow url(...) style values.

Allows sanitized URLs for CSS properties. These can be abused for information
leakage, but only if the CSS rules are already set up to allow for it. That is,
an attacker cannot cause information leakage without controlling the style rules
present, or a very particular setup.

Fixes #8514.
---
 .../src/security/style_sanitizer.ts           | 34 ++++++++++++++++---
 .../test/security/style_sanitizer_spec.ts     |  5 +++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/modules/@angular/platform-browser/src/security/style_sanitizer.ts b/modules/@angular/platform-browser/src/security/style_sanitizer.ts
index 6de0692670..fc145f3914 100644
--- a/modules/@angular/platform-browser/src/security/style_sanitizer.ts
+++ b/modules/@angular/platform-browser/src/security/style_sanitizer.ts
@@ -1,6 +1,8 @@
 import {getDOM} from '../dom/dom_adapter';
 import {assertionsEnabled} from '../../src/facade/lang';
 
+import {sanitizeUrl} from './url_sanitizer';
+
 /**
  * Regular expression for safe style values.
  *
@@ -19,10 +21,29 @@ 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');
 
+/**
+ * Matches a `url(...)` value with an arbitrary argument as long as it does
+ * not contain parentheses.
+ *
+ * The URL value still needs to be sanitized separately.
+ *
+ * `url(...)` values are a very common use case, e.g. for `background-image`. With carefully crafted
+ * CSS style rules, it is possible to construct an information leak with `url` values in CSS, e.g.
+ * by observing whether scroll bars are displayed, or character ranges used by a font face
+ * definition.
+ *
+ * Angular only allows binding CSS values (as opposed to entire CSS rules), so it is unlikely that
+ * binding a URL value without further cooperation from the page will cause an information leak, and
+ * if so, it is just a leak, not a full blown XSS vulnerability.
+ *
+ * Given the common use case, low likelihood of attack vector, and low impact of an attack, this
+ * code is permissive and allows URLs that sanitize otherwise.
+ */
+const URL_RE = /^url\(([^)]+)\)$/;
+
 /**
  * Checks that quotes (" and ') are properly balanced inside a string. Assumes
  * that neither escape (\) nor any other character that could result in
@@ -51,11 +72,16 @@ function hasBalancedQuotes(value: string) {
  */
 export function sanitizeStyle(value: string): string {
   value = String(value).trim();  // 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);
+  // Single url(...) values are supported, but only for URLs that sanitize cleanly. See above for
+  // reasoning behind this.
+  let urlMatch = URL_RE.exec(value);
+  if ((urlMatch && sanitizeUrl(urlMatch[1]) === urlMatch[1]) ||
+      value.match(SAFE_STYLE_VALUE) && hasBalancedQuotes(value)) {
+    return value;  // Safe style values.
   }
 
+  if (assertionsEnabled()) getDOM().log('WARNING: sanitizing unsafe style value ' + value);
+
   return 'unsafe';
 }
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 631889566b..6f27af93d8 100644
--- a/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts
+++ b/modules/@angular/platform-browser/test/security/style_sanitizer_spec.ts
@@ -30,5 +30,10 @@ export function main() {
       expectSanitize('translateX(12px, -5px)').toEqual('translateX(12px, -5px)');
       expectSanitize('scale3d(1, 1, 2)').toEqual('scale3d(1, 1, 2)');
     });
+    t.it('sanitizes URLs', () => {
+      expectSanitize('url(foo/bar.png)').toEqual('url(foo/bar.png)');
+      expectSanitize('url(javascript:evil())').toEqual('unsafe');
+      expectSanitize('url(strangeprotocol:evil)').toEqual('unsafe');
+    });
   });
 }