diff --git a/IPython/html/services/contents/fileio.py b/IPython/html/services/contents/fileio.py
index 77c39c2..0f7ff5c 100644
--- a/IPython/html/services/contents/fileio.py
+++ b/IPython/html/services/contents/fileio.py
@@ -96,8 +96,16 @@ class FileManagerMixin(object):
-------
path : string
Native, absolute OS path to for a file.
+
+ Raises
+ ------
+ 404: if path is outside root
"""
- return to_os_path(path, self.root_dir)
+ root = os.path.abspath(self.root_dir)
+ os_path = to_os_path(path, root)
+ if not (os.path.abspath(os_path) + os.path.sep).startswith(root):
+ raise HTTPError(404, "%s is outside root contents directory" % path)
+ return os_path
def _read_notebook(self, os_path, as_version=4):
"""Read a notebook from an os path."""
diff --git a/IPython/html/services/contents/filemanager.py b/IPython/html/services/contents/filemanager.py
index f775ba8..01ce07b 100644
--- a/IPython/html/services/contents/filemanager.py
+++ b/IPython/html/services/contents/filemanager.py
@@ -373,10 +373,11 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
if 'content' not in model and model['type'] != 'directory':
raise web.HTTPError(400, u'No file content provided')
- self.run_pre_save_hook(model=model, path=path)
-
os_path = self._get_os_path(path)
self.log.debug("Saving %s", os_path)
+
+ self.run_pre_save_hook(model=model, path=path)
+
try:
if model['type'] == 'notebook':
nb = nbformat.from_dict(model['content'])
diff --git a/IPython/html/services/contents/tests/test_manager.py b/IPython/html/services/contents/tests/test_manager.py
index ad64ada..840d10d 100644
--- a/IPython/html/services/contents/tests/test_manager.py
+++ b/IPython/html/services/contents/tests/test_manager.py
@@ -5,6 +5,7 @@ from __future__ import print_function
import os
import sys
import time
+from contextlib import contextmanager
from nose import SkipTest
from tornado.web import HTTPError
@@ -164,7 +165,17 @@ class TestContentsManager(TestCase):
def tearDown(self):
self._temp_dir.cleanup()
-
+
+ @contextmanager
+ def assertRaisesHTTPError(self, status, msg=None):
+ msg = msg or "Should have raised HTTPError(%i)" % status
+ try:
+ yield
+ except HTTPError as e:
+ self.assertEqual(e.status_code, status)
+ else:
+ self.fail(msg)
+
def make_dir(self, api_path):
"""make a subdirectory at api_path
@@ -461,4 +472,29 @@ class TestContentsManager(TestCase):
cm.mark_trusted_cells(nb, path)
cm.check_and_sign(nb, path)
assert cm.notary.check_signature(nb)
+
+ def test_escape_root(self):
+ cm = self.contents_manager
+ # make foo, bar next to root
+ with open(os.path.join(cm.root_dir, '..', 'foo'), 'w') as f:
+ f.write('foo')
+ with open(os.path.join(cm.root_dir, '..', 'bar'), 'w') as f:
+ f.write('bar')
+
+ with self.assertRaisesHTTPError(404):
+ cm.get('..')
+ with self.assertRaisesHTTPError(404):
+ cm.get('foo/../../../bar')
+ with self.assertRaisesHTTPError(404):
+ cm.delete('../foo')
+ with self.assertRaisesHTTPError(404):
+ cm.rename('../foo', '../bar')
+ with self.assertRaisesHTTPError(404):
+ cm.save(model={
+ 'type': 'file',
+ 'content': u'',
+ 'format': 'text',
+ }, path='../foo')
+
+