From f937df9456deac1f607c8f779739b91c7db7e8f5 Mon Sep 17 00:00:00 2001
From: Angus Salkeld <asalkeld@mirantis.com>
Date: Thu, 17 Sep 2015 13:03:16 +1000
Subject: [PATCH] Restrict template file access to /etc/heat/templates/

Closes-bug: 1496277
Change-Id: I85613410477392df144f9573b753b8126445367b
---
 heat/common/urlfetch.py     |  5 +++++
 heat/tests/test_urlfetch.py | 27 +++++++++++++++++++--------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/heat/common/urlfetch.py b/heat/common/urlfetch.py
index 9cec563..0088857 100644
--- a/heat/common/urlfetch.py
+++ b/heat/common/urlfetch.py
@@ -13,6 +13,8 @@
 
 """Utility for fetching a resource (e.g. a template) from a URL."""
 
+import os.path
+
 from oslo_config import cfg
 from oslo_log import log as logging
 import requests
@@ -48,6 +50,9 @@ def get(url, allowed_schemes=('http', 'https')):
         raise URLFetchError(_('Invalid URL scheme %s') % components.scheme)
 
     if components.scheme == 'file':
+        npath = os.path.normpath(components.path)
+        if not npath.startswith('/etc/heat/templates/'):
+            raise URLFetchError(_('Access denied'))
         try:
             return urllib.request.urlopen(url).read()
         except urllib.error.URLError as uex:
diff --git a/heat/tests/test_urlfetch.py b/heat/tests/test_urlfetch.py
index 47bcb88b..24d0230 100644
--- a/heat/tests/test_urlfetch.py
+++ b/heat/tests/test_urlfetch.py
@@ -36,38 +36,49 @@ class Response(object):
 class UrlFetchTest(common.HeatTestCase):
     def setUp(self):
         super(UrlFetchTest, self).setUp()
+        self.file_url = 'file:///etc/heat/templates/a.yaml'
         self.m.StubOutWithMock(requests, 'get')
 
     def test_file_scheme_default_behaviour(self):
         self.m.ReplayAll()
         self.assertRaises(urlfetch.URLFetchError,
-                          urlfetch.get, 'file:///etc/profile')
+                          urlfetch.get, self.file_url)
         self.m.VerifyAll()
 
     def test_file_scheme_supported(self):
         data = '{ "foo": "bar" }'
-        url = 'file:///etc/profile'
 
         self.m.StubOutWithMock(six.moves.urllib.request, 'urlopen')
-        six.moves.urllib.request.urlopen(url).AndReturn(
+        six.moves.urllib.request.urlopen(self.file_url).AndReturn(
             six.moves.cStringIO(data))
         self.m.ReplayAll()
 
-        self.assertEqual(data, urlfetch.get(url, allowed_schemes=['file']))
+        self.assertEqual(data, urlfetch.get(self.file_url,
+                                            allowed_schemes=['file']))
         self.m.VerifyAll()
 
     def test_file_scheme_failure(self):
-        url = 'file:///etc/profile'
-
         self.m.StubOutWithMock(six.moves.urllib.request, 'urlopen')
-        six.moves.urllib.request.urlopen(url).AndRaise(
+        six.moves.urllib.request.urlopen(self.file_url).AndRaise(
             six.moves.urllib.error.URLError('oops'))
         self.m.ReplayAll()
 
         self.assertRaises(urlfetch.URLFetchError,
-                          urlfetch.get, url, allowed_schemes=['file'])
+                          urlfetch.get, self.file_url,
+                          allowed_schemes=['file'])
         self.m.VerifyAll()
 
+    def test_file_bad_path(self):
+        bad_paths = ['file:///etc/heat/heat.conf',
+                     'file:///foo.yaml',
+                     'file:///dev/zero#/etc/heat/templates/a.yaml',
+                     'file:///etc/heat/templates/../../../dev/zero#a.yaml']
+        for bad_path in bad_paths:
+            exc = self.assertRaises(urlfetch.URLFetchError,
+                                    urlfetch.get, bad_path,
+                                    allowed_schemes=['file'])
+            self.assertIn('Access denied', six.text_type(exc))
+
     def test_http_scheme(self):
         url = 'http://example.com/template'
         data = '{ "foo": "bar" }'
-- 
2.1.4