From cf9bad7bb83e9f2de924ede4125542abe79d1296 Mon Sep 17 00:00:00 2001 From: Alessio Fabiani Date: Thu, 22 Dec 2022 11:57:55 +0100 Subject: [PATCH] [Fixes #10464] Fix code scanning alert - Uncontrolled data used in path expression (#10465) * [Fixes #10462] GeoNode is vulnerable to an XML External Entity (XXE) injection * [Fixes #10464] Fix code scanning alert - Uncontrolled data used in path expression --- geonode/geoserver/tests/test_helpers.py | 35 ++++++++++++++++++++++++- geonode/geoserver/views.py | 4 ++- geonode/utils.py | 21 +++++++++++++++ requirements.txt | 1 + setup.cfg | 1 + 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/geonode/geoserver/tests/test_helpers.py b/geonode/geoserver/tests/test_helpers.py index 228324f9b79..3b2fbeb7cb9 100644 --- a/geonode/geoserver/tests/test_helpers.py +++ b/geonode/geoserver/tests/test_helpers.py @@ -17,14 +17,17 @@ # ######################################################################### import re +import os import logging from urllib.parse import urljoin +from pathvalidate import ValidationError from django.conf import settings from django.urls import reverse -from geonode import geoserver +from geonode import geoserver, GeoNodeException +from geonode.utils import safe_path_leaf from geonode.decorators import on_ogc_backend from geonode.tests.base import GeoNodeBaseTestSupport from geonode.geoserver.views import _response_callback @@ -87,6 +90,36 @@ def test_extract_name_from_sld(self): """ self.assertIsNone(extract_name_from_sld(gs_catalog, content)) + @on_ogc_backend(geoserver.BACKEND_PACKAGE) + def test_safe_path_leaf(self): + base_path = settings.MEDIA_ROOT + + malformed_paths = [ + 'c:/etc/passwd', + 'c:\\etc\\passwd', + '\0_a*b:ce%f/(g)h+i_0.txt' + ] + for _path in malformed_paths: + with self.assertRaises(ValidationError): + safe_path_leaf(_path) + + unsafe_paths = [ + '/root/', + '~/.ssh', + '$HOME/.ssh', + '/etc/passwd', + '.../style.sld', + 'fi:l*e/p"a?t>h|.t diff --git a/geonode/geoserver/views.py b/geonode/geoserver/views.py index 642dcf0a956..c9830a07574 100644 --- a/geonode/geoserver/views.py +++ b/geonode/geoserver/views.py @@ -63,6 +63,7 @@ json_response, _get_basic_auth_info, http_client, + safe_path_leaf, get_headers, get_dataset_workspace) from geoserver.catalog import FailedRequestError @@ -175,7 +176,8 @@ def respond(*args, **kw): try: # Check SLD is valid try: - if sld: + _allowed_sld_extensions = ['.sld', '.xml', '.css', '.txt', '.yml'] + if sld and os.path.splitext(safe_path_leaf(sld))[1].lower() in _allowed_sld_extensions: if isfile(sld): with open(sld) as sld_file: sld = sld_file.read() diff --git a/geonode/utils.py b/geonode/utils.py index 6521b70d059..6c38fa6fc29 100755 --- a/geonode/utils.py +++ b/geonode/utils.py @@ -24,6 +24,7 @@ import json import time import base64 +import ntpath import select import shutil import string @@ -50,6 +51,7 @@ from rest_framework.exceptions import APIException from math import atan, exp, log, pi, sin, tan, floor from zipfile import ZipFile, is_zipfile, ZIP_DEFLATED +from pathvalidate import ValidationError, validate_filepath, validate_filename from geonode.upload.api.exceptions import GeneralUploadException from django.conf import settings @@ -1905,3 +1907,22 @@ def get_supported_datasets_file_types(): def get_allowed_extensions(): return list(itertools.chain.from_iterable([_type['ext'] for _type in get_supported_datasets_file_types()])) + + +def safe_path_leaf(path): + """A view that is not vulnerable to malicious file access.""" + base_path = settings.MEDIA_ROOT + try: + validate_filepath(path, platform='auto') + head, tail = ntpath.split(path) + filename = tail or ntpath.basename(head) + validate_filename(filename, platform='auto') + except ValidationError as e: + logger.error(f"{e}") + raise e + # GOOD -- Verify with normalised version of path + fullpath = os.path.normpath(os.path.join(head, filename)) + if not fullpath.startswith(base_path) or path != fullpath: + raise GeoNodeException( + f"The provided path '{path}' is not safe. The file is outside the MEDIA_ROOT '{base_path}' base path!") + return fullpath diff --git a/requirements.txt b/requirements.txt index e7a90fca94f..366539fbe44 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,6 +26,7 @@ schema==0.7.5 rdflib==6.1.1 smart_open==6.2.0 PyMuPDF==1.21.1 +pathvalidate==2.5.2 # Django Apps django-allauth==0.51.0 diff --git a/setup.cfg b/setup.cfg index e71f2c70ebc..e45d78cc212 100644 --- a/setup.cfg +++ b/setup.cfg @@ -52,6 +52,7 @@ install_requires = rdflib==6.1.1 smart_open==6.3.0 PyMuPDF==1.21.1 + pathvalidate==2.5.2 # Django Apps django-allauth==0.51.0