diff --git a/IPython/html/services/contents/fileio.py b/IPython/html/services/contents/fileio.py index 77c39c2f5..0f7ff5c7a 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 f775ba89f..01ce07b64 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 ad64ada0b..840d10db0 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') + +