From 54ec015f720a4379e8ffc34345b3a7bf36b6f15b Mon Sep 17 00:00:00 2001
From: CristianFiorentino <cristian.fiorentino@intel.com>
Date: Mon, 10 Mar 2014 17:36:31 -0300
Subject: [PATCH] Introduces escaping in Horizon/Orchestration

1) Escape help_text a second time to avoid bootstrap tooltip XSS issue

The "Description" parameter in a Heat template is used to populate
a help_text tooltip in the dynamically generated Heat form. Bootstrap
inserts this tooltip into the DOM using .html() which undoes any
escaping we do in Django (it should be using .text()).

This was fixed by forcing the help_text content to be escaped a second
time. The issue itself is mitigated in bootstrap.js release 2.0.3
(ours is currently 2.0.1).

2) Properly escape untrusted Heat template 'outputs'

The 'outputs' parameter in a Heat template was included in a Django
template with HTML autoescaping turned off. Malicious HTML content
could be included in a Heat template and would be rendered by Horizon
when details about a created stack were displayed.

This was fixed by not disabling autoescaping and explicitly escaping
untrusted values in any strings that are later marked "safe" to render
without further escaping.

Conflicts:
	openstack_dashboard/dashboards/project/stacks/mappings.py

Change-Id: Icd9f9d9ca77068b12227d77469773a325c840001
Closes-Bug: #1289033
Co-Authored-By: Kieran Spear <kispear@gmail.com>
---
 .../templates/horizon/common/_form_fields.html  |  7 ++++++-
 .../dashboards/project/stacks/mappings.py       | 10 ++++++++--
 .../templates/stacks/_detail_overview.html      |  3 +--
 .../dashboards/project/stacks/tests.py          | 17 +++++++++++------
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/horizon/templates/horizon/common/_form_fields.html b/horizon/templates/horizon/common/_form_fields.html
index 35676142a7..f6fb98fc93 100644
--- a/horizon/templates/horizon/common/_form_fields.html
+++ b/horizon/templates/horizon/common/_form_fields.html
@@ -14,7 +14,12 @@
         <span class="help-inline">{{ error }}</span>
       {% endfor %}
     {% endif %}
-    <span class="help-block">{{ field.help_text }}</span>
+    {% comment %}
+    Escape help_text a second time here, to avoid an XSS issue in bootstrap.js.
+    This can most likely be removed once we upgrade bootstrap.js past 2.0.2.
+    Note: the spaces are necessary here.
+    {% endcomment %}
+    <span class="help-block">{% filter force_escape %} {{ field.help_text }} {% endfilter %} </span>
     <div class="input">
       {{ field }}
     </div>
diff --git a/openstack_dashboard/dashboards/project/stacks/mappings.py b/openstack_dashboard/dashboards/project/stacks/mappings.py
index 0353291248..f1389c5f06 100644
--- a/openstack_dashboard/dashboards/project/stacks/mappings.py
+++ b/openstack_dashboard/dashboards/project/stacks/mappings.py
@@ -19,6 +19,8 @@ import urlparse
 
 from django.core.urlresolvers import reverse  # noqa
 from django.template.defaultfilters import register  # noqa
+from django.utils import html
+from django.utils import safestring
 
 from openstack_dashboard.api import swift
 
@@ -76,11 +78,15 @@ def stack_output(output):
     if not output:
         return u''
     if isinstance(output, dict) or isinstance(output, list):
-        return u'<pre>%s</pre>' % json.dumps(output, indent=2)
+        json_string = json.dumps(output, indent=2)
+        safe_output = u'<pre>%s</pre>' % html.escape(json_string)
+        return safestring.mark_safe(safe_output)
     if isinstance(output, basestring):
         parts = urlparse.urlsplit(output)
         if parts.netloc and parts.scheme in ('http', 'https'):
-            return u'<a href="%s" target="_blank">%s</a>' % (output, output)
+            url = html.escape(output)
+            safe_link = u'<a href="%s" target="_blank">%s</a>' % (url, url)
+            return safestring.mark_safe(safe_link)
     return unicode(output)
 
 
diff --git a/openstack_dashboard/dashboards/project/stacks/templates/stacks/_detail_overview.html b/openstack_dashboard/dashboards/project/stacks/templates/stacks/_detail_overview.html
index f4756e07aa..33fe78309c 100644
--- a/openstack_dashboard/dashboards/project/stacks/templates/stacks/_detail_overview.html
+++ b/openstack_dashboard/dashboards/project/stacks/templates/stacks/_detail_overview.html
@@ -36,9 +36,8 @@
     <dt>{{ output.output_key }}</dt>
     <dd>{{ output.description }}</dd>
     <dd>
-    {% autoescape off %}
     {{ output.output_value|stack_output }}
-    {% endautoescape %}</dd>
+    </dd>
     {% endfor %}
   </dl>
 </div>
diff --git a/openstack_dashboard/dashboards/project/stacks/tests.py b/openstack_dashboard/dashboards/project/stacks/tests.py
index 408d86fc17..986e3e0db6 100644
--- a/openstack_dashboard/dashboards/project/stacks/tests.py
+++ b/openstack_dashboard/dashboards/project/stacks/tests.py
@@ -16,6 +16,7 @@ import json
 
 from django.core.urlresolvers import reverse  # noqa
 from django import http
+from django.utils import html
 
 from mox import IsA  # noqa
 
@@ -77,12 +78,16 @@ class MappingsTests(test.TestCase):
         self.assertEqual(u'foo', mappings.stack_output('foo'))
         self.assertEqual(u'', mappings.stack_output(None))
 
-        self.assertEqual(
-            u'<pre>[\n  "one", \n  "two", \n  "three"\n]</pre>',
-            mappings.stack_output(['one', 'two', 'three']))
-        self.assertEqual(
-            u'<pre>{\n  "foo": "bar"\n}</pre>',
-            mappings.stack_output({'foo': 'bar'}))
+        outputs = ['one', 'two', 'three']
+        expected_text = """[\n  "one", \n  "two", \n  "three"\n]"""
+
+        self.assertEqual(u'<pre>%s</pre>' % html.escape(expected_text),
+                         mappings.stack_output(outputs))
+
+        outputs = {'foo': 'bar'}
+        expected_text = """{\n  "foo": "bar"\n}"""
+        self.assertEqual(u'<pre>%s</pre>' % html.escape(expected_text),
+                         mappings.stack_output(outputs))
 
         self.assertEqual(
             u'<a href="http://www.example.com/foo" target="_blank">'