From 7a524e3debf85f3cb773e7d4a8914d625ddb256e Mon Sep 17 00:00:00 2001
From: Martin Probst <martin@probst.io>
Date: Tue, 3 May 2016 18:41:31 -0700
Subject: [PATCH] feat(security): add tests for URL sanitization.

---
 .../src/security/html_sanitizer.ts            | 11 +++++
 .../test/security/url_sanitizer_spec.ts       | 47 +++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts

diff --git a/modules/@angular/platform-browser/src/security/html_sanitizer.ts b/modules/@angular/platform-browser/src/security/html_sanitizer.ts
index 4059c9f0b2..74a8779739 100644
--- a/modules/@angular/platform-browser/src/security/html_sanitizer.ts
+++ b/modules/@angular/platform-browser/src/security/html_sanitizer.ts
@@ -89,6 +89,14 @@ const HTML_ATTRS =
            'scope,scrolling,shape,size,span,start,summary,tabindex,target,title,type,' +
            'valign,value,vspace,width');
 
+// NB: This currently conciously doesn't support SVG. SVG sanitization has had several security
+// issues in the past, so it seems safer to leave it out if possible. If support for binding SVG via
+// innerHTML is required, SVG attributes should be added here.
+
+// NB: Sanitization does not allow <form> elements or other active elements (<button> etc). Those
+// can be sanitized, but they increase security surface area without a legitimate use case, so they
+// are left out here.
+
 const VALID_ATTRS = merge(URI_ATTRS, HTML_ATTRS);
 
 /**
@@ -99,6 +107,9 @@ class SanitizingHtmlSerializer {
   private buf: string[] = [];
 
   sanitizeChildren(el: Element): string {
+    // This cannot use a TreeWalker, as it has to run on Angular's various DOM adapters.
+    // However this code never accesses properties off of `document` before deleting its contents
+    // again, so it shouldn't be vulnerable to DOM clobbering.
     let current: Node = el.firstChild;
     while (current) {
       if (DOM.isElementNode(current)) {
diff --git a/modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts b/modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts
new file mode 100644
index 0000000000..22da293932
--- /dev/null
+++ b/modules/@angular/platform-browser/test/security/url_sanitizer_spec.ts
@@ -0,0 +1,47 @@
+import * as t from '@angular/core/testing/testing_internal';
+import {sanitizeUrl} from '../../src/security/url_sanitizer';
+
+export function main() {
+  t.describe('URL sanitizer', () => {
+    t.describe('valid URLs', () => {
+      const validUrls = [
+        '',
+        'http://abc',
+        'HTTP://abc',
+        'https://abc',
+        'HTTPS://abc',
+        'ftp://abc',
+        'FTP://abc',
+        'mailto:me@example.com',
+        'MAILTO:me@example.com',
+        'tel:123-123-1234',
+        'TEL:123-123-1234',
+        '#anchor',
+        '/page1.md',
+        'http://JavaScript/my.js'
+      ];
+      for (let url of validUrls) {
+        t.it(`valid ${url}`, () => t.expect(sanitizeUrl(url)).toEqual(url));
+      }
+    });
+
+    t.describe('invalid URLs', () => {
+      const invalidUrls = [
+        'javascript:evil()',
+        'JavaScript:abc',
+        'evilNewProtocol:abc',
+        ' \n Java\n Script:abc',
+        '&#106;&#97;&#118;&#97;&#115;&#99;&#114;&#105;&#112;&#116;&#58;',
+        '&#106&#97;&#118;&#97;&#115;&#99;&#114;&#105;&#112;&#116;&#58;',
+        '&#106 &#97;&#118;&#97;&#115;&#99;&#114;&#105;&#112;&#116;&#58;',
+        '&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058',
+        '&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A;',
+        'jav&#x09;ascript:alert();',
+        'jav\u0000ascript:alert();',
+      ];
+      for (let url of invalidUrls) {
+        t.it(`valid ${url}`, () => t.expect(sanitizeUrl(url)).toMatch(/^unsafe:/));
+      }
+    });
+  });
+}