mirror of
https://github.com/GNS3/gns3-server
synced 2024-11-18 06:18:08 +00:00
Merge pull request #1900 from GNS3/prevent-directory-traversal
Prevent directory traversal
This commit is contained in:
commit
3a479d7ea6
@ -481,7 +481,7 @@ class BaseManager:
|
||||
|
||||
if not path:
|
||||
return ""
|
||||
orig_path = path
|
||||
orig_path = os.path.normpath(path)
|
||||
|
||||
server_config = self.config.get_section_config("Server")
|
||||
img_directory = self.get_images_directory()
|
||||
@ -494,7 +494,8 @@ class BaseManager:
|
||||
if re.match(r"^[A-Z]:", path) is not None:
|
||||
raise NodeError("{} is not allowed on this remote server. Please only use a file from '{}'".format(path, img_directory))
|
||||
|
||||
if not os.path.isabs(path):
|
||||
if not os.path.isabs(orig_path):
|
||||
|
||||
for directory in valid_directory_prefices:
|
||||
log.debug("Searching for image '{}' in '{}'".format(orig_path, directory))
|
||||
path = self._recursive_search_file_in_directory(directory, orig_path)
|
||||
@ -503,6 +504,12 @@ class BaseManager:
|
||||
|
||||
# Not found we try the default directory
|
||||
log.debug("Searching for image '{}' in default directory".format(orig_path))
|
||||
|
||||
# check that the image path is in the default image directory
|
||||
#common_prefix = os.path.commonprefix([orig_path, img_directory])
|
||||
#if common_prefix != img_directory:
|
||||
# raise NodeError("{} is not allowed. Please only use a file from '{}'".format(orig_path, img_directory))
|
||||
|
||||
s = os.path.split(orig_path)
|
||||
path = force_unix_path(os.path.join(img_directory, *s))
|
||||
if os.path.exists(path):
|
||||
@ -517,7 +524,6 @@ class BaseManager:
|
||||
return path
|
||||
raise ImageMissingError(orig_path)
|
||||
|
||||
# Check to see if path is an absolute path to a valid directory
|
||||
path = force_unix_path(path)
|
||||
for directory in valid_directory_prefices:
|
||||
log.debug("Searching for image '{}' in '{}'".format(orig_path, directory))
|
||||
@ -525,7 +531,8 @@ class BaseManager:
|
||||
if os.path.exists(path):
|
||||
return path
|
||||
raise ImageMissingError(orig_path)
|
||||
raise NodeError("{} is not allowed on this remote server. Please only use a file from '{}'".format(path, img_directory))
|
||||
raise NodeError("{} is not allowed on this remote server. Please only use a file from '{}'"
|
||||
.format(path, img_directory))
|
||||
|
||||
def _recursive_search_file_in_directory(self, directory, searched_file):
|
||||
"""
|
||||
@ -533,12 +540,12 @@ class BaseManager:
|
||||
|
||||
:returns: Path or None if not found
|
||||
"""
|
||||
s = os.path.split(searched_file)
|
||||
|
||||
s = os.path.split(searched_file)
|
||||
for root, dirs, files in os.walk(directory):
|
||||
for file in files:
|
||||
# If filename is the same
|
||||
if s[1] == file and (s[0] == '' or s[0] == os.path.basename(root)):
|
||||
if s[1] == file and (s[0] == '' or os.path.basename(s[0]) == os.path.basename(root)):
|
||||
path = os.path.normpath(os.path.join(root, s[1]))
|
||||
if os.path.exists(path):
|
||||
return path
|
||||
|
@ -484,15 +484,15 @@ class DynamipsVMHandler:
|
||||
raw=True,
|
||||
description="Download a Dynamips IOS image")
|
||||
async def download_image(request, response):
|
||||
|
||||
filename = request.match_info["filename"]
|
||||
|
||||
# Raise error if user try to escape
|
||||
if filename[0] == "." or os.path.sep in filename:
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
dynamips_manager = Dynamips.instance()
|
||||
image_path = dynamips_manager.get_abs_image_path(filename)
|
||||
|
||||
# Raise error if user try to escape
|
||||
if filename[0] == ".":
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
await response.stream_file(image_path)
|
||||
|
||||
@Route.post(
|
||||
|
@ -443,15 +443,15 @@ class IOUHandler:
|
||||
raw=True,
|
||||
description="Download an IOU image")
|
||||
async def download_image(request, response):
|
||||
|
||||
filename = request.match_info["filename"]
|
||||
|
||||
# Raise error if user try to escape
|
||||
if filename[0] == "." or os.path.sep in filename:
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
iou_manager = IOU.instance()
|
||||
image_path = iou_manager.get_abs_image_path(filename)
|
||||
|
||||
# Raise error if user try to escape
|
||||
if filename[0] == ".":
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
await response.stream_file(image_path)
|
||||
|
||||
@Route.get(
|
||||
|
@ -20,12 +20,12 @@ import asyncio
|
||||
import json
|
||||
import os
|
||||
import psutil
|
||||
import tempfile
|
||||
|
||||
from gns3server.web.route import Route
|
||||
from gns3server.compute.project_manager import ProjectManager
|
||||
from gns3server.compute import MODULES
|
||||
from gns3server.utils.cpu_percent import CpuPercent
|
||||
from gns3server.utils.path import is_safe_path
|
||||
|
||||
from gns3server.schemas.project import (
|
||||
PROJECT_OBJECT_SCHEMA,
|
||||
@ -247,14 +247,13 @@ class ProjectHandler:
|
||||
|
||||
pm = ProjectManager.instance()
|
||||
project = pm.get_project(request.match_info["project_id"])
|
||||
path = request.match_info["path"]
|
||||
path = os.path.normpath(path)
|
||||
path = os.path.normpath(request.match_info["path"])
|
||||
|
||||
# Raise error if user try to escape
|
||||
if path[0] == ".":
|
||||
if not is_safe_path(path, project.path):
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
path = os.path.join(project.path, path)
|
||||
|
||||
path = os.path.join(project.path, path)
|
||||
await response.stream_file(path)
|
||||
|
||||
@Route.post(
|
||||
@ -273,15 +272,14 @@ class ProjectHandler:
|
||||
|
||||
pm = ProjectManager.instance()
|
||||
project = pm.get_project(request.match_info["project_id"])
|
||||
path = request.match_info["path"]
|
||||
path = os.path.normpath(path)
|
||||
path = os.path.normpath(request.match_info["path"])
|
||||
|
||||
# Raise error if user try to escape
|
||||
if path[0] == ".":
|
||||
if not is_safe_path(path, project.path):
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
path = os.path.join(project.path, path)
|
||||
|
||||
response.set_status(200)
|
||||
path = os.path.join(project.path, path)
|
||||
response.set_status(201)
|
||||
|
||||
try:
|
||||
os.makedirs(os.path.dirname(path), exist_ok=True)
|
||||
|
@ -566,15 +566,15 @@ class QEMUHandler:
|
||||
raw=True,
|
||||
description="Download Qemu image")
|
||||
async def download_image(request, response):
|
||||
|
||||
filename = request.match_info["filename"]
|
||||
|
||||
# Raise error if user try to escape
|
||||
if filename[0] == "." or os.path.sep in filename:
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
qemu_manager = Qemu.instance()
|
||||
image_path = qemu_manager.get_abs_image_path(filename)
|
||||
|
||||
# Raise error if user try to escape
|
||||
if filename[0] == ".":
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
await response.stream_file(image_path)
|
||||
|
||||
@Route.get(
|
||||
|
@ -409,21 +409,19 @@ class NodeHandler:
|
||||
path = request.match_info["path"]
|
||||
path = force_unix_path(path)
|
||||
|
||||
|
||||
# Raise error if user try to escape
|
||||
if path[0] == ".":
|
||||
if path[0] == "." or "/../" in path:
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
node_type = node.node_type
|
||||
path = "/project-files/{}/{}/{}".format(node_type, node.id, path)
|
||||
|
||||
res = await node.compute.http_query("GET", "/projects/{project_id}/files{path}".format(project_id=project.id, path=path), timeout=None, raw=True)
|
||||
response.set_status(200)
|
||||
response.content_type = "application/octet-stream"
|
||||
response.enable_chunked_encoding()
|
||||
await response.prepare(request)
|
||||
await response.write(res.body)
|
||||
# await response.write_eof() #FIXME: shound't be needed anymore
|
||||
response.set_status(res.status)
|
||||
if res.status == 200:
|
||||
response.content_type = "application/octet-stream"
|
||||
response.enable_chunked_encoding()
|
||||
await response.prepare(request)
|
||||
await response.write(res.body)
|
||||
|
||||
@Route.post(
|
||||
r"/projects/{project_id}/nodes/{node_id}/files/{path:.+}",
|
||||
@ -446,14 +444,14 @@ class NodeHandler:
|
||||
path = force_unix_path(path)
|
||||
|
||||
# Raise error if user try to escape
|
||||
if path[0] == ".":
|
||||
if path[0] == "." or "/../" in path:
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
node_type = node.node_type
|
||||
path = "/project-files/{}/{}/{}".format(node_type, node.id, path)
|
||||
data = await request.content.read() #FIXME: are we handling timeout or large files correctly?
|
||||
await node.compute.http_query("POST", "/projects/{project_id}/files{path}".format(project_id=project.id, path=path), data=data, timeout=None, raw=True)
|
||||
response.set_status(201)
|
||||
res = await node.compute.http_query("POST", "/projects/{project_id}/files{path}".format(project_id=project.id, path=path), data=data, timeout=None, raw=True)
|
||||
response.set_status(res.status)
|
||||
|
||||
@Route.get(
|
||||
r"/projects/{project_id}/nodes/{node_id}/console/ws",
|
||||
|
@ -28,6 +28,7 @@ from gns3server.controller import Controller
|
||||
from gns3server.controller.import_project import import_project
|
||||
from gns3server.controller.export_project import export_project
|
||||
from gns3server.utils.asyncio import aiozipstream
|
||||
from gns3server.utils.path import is_safe_path
|
||||
from gns3server.config import Config
|
||||
|
||||
|
||||
@ -454,14 +455,12 @@ class ProjectHandler:
|
||||
|
||||
controller = Controller.instance()
|
||||
project = await controller.get_loaded_project(request.match_info["project_id"])
|
||||
path = request.match_info["path"]
|
||||
path = os.path.normpath(path).strip('/')
|
||||
path = os.path.normpath(request.match_info["path"])
|
||||
|
||||
# Raise error if user try to escape
|
||||
if path[0] == ".":
|
||||
if not is_safe_path(path, project.path):
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
path = os.path.join(project.path, path)
|
||||
|
||||
await response.stream_file(path)
|
||||
|
||||
@Route.post(
|
||||
@ -480,15 +479,13 @@ class ProjectHandler:
|
||||
|
||||
controller = Controller.instance()
|
||||
project = await controller.get_loaded_project(request.match_info["project_id"])
|
||||
path = request.match_info["path"]
|
||||
path = os.path.normpath(path).strip("/")
|
||||
path = os.path.normpath(request.match_info["path"])
|
||||
|
||||
# Raise error if user try to escape
|
||||
if path[0] == ".":
|
||||
if not is_safe_path(path, project.path):
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
path = os.path.join(project.path, path)
|
||||
|
||||
response.set_status(200)
|
||||
response.set_status(201)
|
||||
|
||||
try:
|
||||
async with aiofiles.open(path, 'wb+') as f:
|
||||
|
@ -92,7 +92,7 @@ class IndexHandler:
|
||||
filename = os.path.join('static', 'web-ui', filename)
|
||||
|
||||
# Raise error if user try to escape
|
||||
if filename[0] == ".":
|
||||
if filename[0] == "." or '/../' in filename:
|
||||
raise aiohttp.web.HTTPForbidden()
|
||||
|
||||
static = get_resource(filename)
|
||||
|
@ -37,6 +37,17 @@ def get_default_project_directory():
|
||||
return path
|
||||
|
||||
|
||||
def is_safe_path(file_path, directory):
|
||||
"""
|
||||
Check that file path is safe.
|
||||
(the file is stored inside directory or one of its sub-directory)
|
||||
"""
|
||||
|
||||
requested_path = os.path.abspath(file_path)
|
||||
common_prefix = os.path.commonprefix([requested_path, directory])
|
||||
return common_prefix != directory
|
||||
|
||||
|
||||
def check_path_allowed(path):
|
||||
"""
|
||||
If the server is non local raise an error if
|
||||
|
@ -172,7 +172,7 @@ async def test_write_file(compute_api, tmpdir):
|
||||
project = ProjectManager.instance().create_project(project_id="01010203-0405-0607-0809-0a0b0c0d0e0b")
|
||||
|
||||
response = await compute_api.post("/projects/{project_id}/files/hello".format(project_id=project.id), body="world", raw=True)
|
||||
assert response.status == 200
|
||||
assert response.status == 201
|
||||
|
||||
with open(os.path.join(project.path, "hello")) as f:
|
||||
assert f.read() == "world"
|
||||
|
@ -108,7 +108,7 @@ async def test_qemu_create_with_params(compute_api, compute_project, base_params
|
||||
async def test_qemu_create_with_project_file(compute_api, compute_project, base_params, fake_qemu_vm):
|
||||
|
||||
response = await compute_api.post("/projects/{project_id}/files/hello.img".format(project_id=compute_project.id), body="world", raw=True)
|
||||
assert response.status == 200
|
||||
assert response.status == 201
|
||||
params = base_params
|
||||
params["hda_disk_image"] = "hello.img"
|
||||
response = await compute_api.post("/projects/{project_id}/qemu/nodes".format(project_id=compute_project.id), params)
|
||||
@ -278,7 +278,6 @@ async def test_images(compute_api, fake_qemu_vm):
|
||||
response = await compute_api.get("/qemu/images")
|
||||
assert response.status == 200
|
||||
assert {"filename": "linux载.img", "path": "linux载.img", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1} in response.json
|
||||
assert {'filename': 'config.img', 'filesize': 1048576, 'md5sum': '0ab49056760ae1db6c25376446190b47', 'path': 'config.img'} in response.json
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform.startswith("win"), reason="Does not work on Windows")
|
||||
|
@ -218,6 +218,7 @@ async def test_get_file(controller_api, project, node, compute):
|
||||
|
||||
response = MagicMock()
|
||||
response.body = b"world"
|
||||
response.status = 200
|
||||
compute.http_query = AsyncioMagicMock(return_value=response)
|
||||
|
||||
response = await controller_api.get("/projects/{project_id}/nodes/{node_id}/files/hello".format(project_id=project.id, node_id=node.id))
|
||||
@ -232,7 +233,9 @@ async def test_get_file(controller_api, project, node, compute):
|
||||
|
||||
async def test_post_file(controller_api, project, node, compute):
|
||||
|
||||
compute.http_query = AsyncioMagicMock()
|
||||
response = MagicMock()
|
||||
response.status = 201
|
||||
compute.http_query = AsyncioMagicMock(return_value=response)
|
||||
response = await controller_api.post("/projects/{project_id}/nodes/{node_id}/files/hello".format(project_id=project.id, node_id=node.id), body=b"hello", raw=True)
|
||||
assert response.status == 201
|
||||
|
||||
@ -247,6 +250,7 @@ async def test_get_and_post_with_nested_paths_normalization(controller_api, proj
|
||||
|
||||
response = MagicMock()
|
||||
response.body = b"world"
|
||||
response.status = 200
|
||||
compute.http_query = AsyncioMagicMock(return_value=response)
|
||||
response = await controller_api.get("/projects/{project_id}/nodes/{node_id}/files/hello\\nested".format(project_id=project.id, node_id=node.id))
|
||||
assert response.status == 200
|
||||
@ -254,7 +258,9 @@ async def test_get_and_post_with_nested_paths_normalization(controller_api, proj
|
||||
|
||||
compute.http_query.assert_called_with("GET", "/projects/{project_id}/files/project-files/vpcs/{node_id}/hello/nested".format(project_id=project.id, node_id=node.id), timeout=None, raw=True)
|
||||
|
||||
compute.http_query = AsyncioMagicMock()
|
||||
response = MagicMock()
|
||||
response.status = 201
|
||||
compute.http_query = AsyncioMagicMock(return_value=response)
|
||||
response = await controller_api.post("/projects/{project_id}/nodes/{node_id}/files/hello\\nested".format(project_id=project.id, node_id=node.id), body=b"hello", raw=True)
|
||||
assert response.status == 201
|
||||
|
||||
|
@ -318,7 +318,7 @@ async def test_get_file(controller_api, project):
|
||||
async def test_write_file(controller_api, project):
|
||||
|
||||
response = await controller_api.post("/projects/{project_id}/files/hello".format(project_id=project.id), body="world", raw=True)
|
||||
assert response.status == 200
|
||||
assert response.status == 201
|
||||
|
||||
with open(os.path.join(project.path, "hello")) as f:
|
||||
assert f.read() == "world"
|
||||
|
Loading…
Reference in New Issue
Block a user