From cb8f8770a2ea447ebd08cddccd7998a2d7bd2915 Mon Sep 17 00:00:00 2001 From: Giacomo Marciani Date: Thu, 5 Jun 2025 17:02:56 -0400 Subject: [PATCH 1/2] [Security] Mitigate the risk of path traversal attacks by adding a request validator that rejects requests containing a 'path' argument matching the unsafe path traversal pattern. --- .../validation/test_api_custom_validators.py | 41 ++++++++++++++++++- api/validation/schemas.py | 4 +- api/validation/validators.py | 25 ++++++++++- 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/api/tests/validation/test_api_custom_validators.py b/api/tests/validation/test_api_custom_validators.py index a9ee0b945..9f1afc0a3 100644 --- a/api/tests/validation/test_api_custom_validators.py +++ b/api/tests/validation/test_api_custom_validators.py @@ -1,7 +1,7 @@ import pytest from marshmallow import ValidationError -from api.validation.validators import size_not_exceeding +from api.validation.validators import size_not_exceeding, is_safe_path def test_size_not_exceeding(): @@ -15,4 +15,41 @@ def test_size_not_exceeding_failing(): test_str_not_exceeding = 'a' * max_size # will produce "aaa...", max_size + 2 with pytest.raises(ValidationError): - size_not_exceeding(test_str_not_exceeding, max_size) \ No newline at end of file + size_not_exceeding(test_str_not_exceeding, max_size) + + +@pytest.mark.parametrize( + "path, expected_result", [ + pytest.param( + "/whatever_api_version/whatever_api_resource", + True, + id="safe api path absolute" + ), + pytest.param( + "whatever_api_version/whatever_api_resource", + True, + id="safe api path relative" + ), + pytest.param( + "/../whatever", + False, + id="unsafe path traversal 1" + ), + pytest.param( + "./../whatever", + False, + id="unsafe path traversal 2" + ), + pytest.param( + "/whatever/../whatever", + False, + id="unsafe path traversal 3" + ), + pytest.param( + "whatever/../whatever", + False, + id="unsafe path traversal 4" + ), + ]) +def test_is_safe_path(path: str, expected_result: bool): + assert is_safe_path(path) == expected_result \ No newline at end of file diff --git a/api/validation/schemas.py b/api/validation/schemas.py index 8a1256701..41152edf4 100644 --- a/api/validation/schemas.py +++ b/api/validation/schemas.py @@ -1,7 +1,7 @@ from marshmallow import Schema, fields, validate, INCLUDE, validates_schema from api.validation.validators import aws_region_validator, is_alphanumeric_with_hyphen, \ - valid_api_log_levels_predicate, size_not_exceeding + valid_api_log_levels_predicate, size_not_exceeding, is_safe_path class EC2ActionSchema(Schema): @@ -119,7 +119,7 @@ class PriceEstimateSchema(Schema): PriceEstimate = PriceEstimateSchema(unknown=INCLUDE) class PCProxyArgsSchema(Schema): - path = fields.String(required=True, validate=validate.Length(max=512)) + path = fields.String(required=True, validate=validate.And(is_safe_path, validate.Length(max=512))) PCProxyArgs = PCProxyArgsSchema(unknown=INCLUDE) diff --git a/api/validation/validators.py b/api/validation/validators.py index 8ccb787d5..1cbbdf9bf 100644 --- a/api/validation/validators.py +++ b/api/validation/validators.py @@ -31,4 +31,27 @@ def size_not_exceeding(data, size): bytes_ = bytes(json.dumps(data), 'utf-8') byte_size = len(bytes_) if byte_size > size: - raise ValidationError(f'Request body exceeded max size of {size} bytes') \ No newline at end of file + raise ValidationError(f'Request body exceeded max size of {size} bytes') + +def is_safe_path(arg: str): + """ + Validates if a given path is safe from path traversal attacks. + + This function checks for the presence of directory traversal patterns + (../ or ..\) in the provided path string. These patterns are commonly + used in path traversal attacks to access files outside the intended + directory. + + Args: + arg (str): The path string to validate. + Examples: + - "/v3/clusters" (safe) + - "v3/clusters" (safe) + "/v3/../../stage/v3/clusters" (safe) + - "../stage/v3/clusters" (unsafe) + + Returns: + bool: True if the path is safe (no traversal patterns) + False if the path contains traversal patterns + """ + return not re.search(r'\.\.[\\/]', arg) \ No newline at end of file From 53be4c28b70665204534bc94f8dd65511359502e Mon Sep 17 00:00:00 2001 From: Giacomo Marciani Date: Mon, 9 Jun 2025 18:22:10 -0400 Subject: [PATCH 2/2] [Validators] Minor improvement to message returned in case of inpout validation errors. --- api/tests/exceptions/test_exception_handlers.py | 4 ++-- api/validation/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/tests/exceptions/test_exception_handlers.py b/api/tests/exceptions/test_exception_handlers.py index e92ceb91c..08cff6493 100644 --- a/api/tests/exceptions/test_exception_handlers.py +++ b/api/tests/exceptions/test_exception_handlers.py @@ -47,11 +47,11 @@ def test_unauthenticated_error_handler(client, app, monkeypatch): def test_validation_error_handler(client, app, monkeypatch): with app.test_request_context('/'): app.preprocess_request() - response, status_code = validation_error_handler(ValidationError('Input validation failed for /manager/ec2_action', data={'field': ['validation-error']})) + response, status_code = validation_error_handler(ValidationError('Input validation failed for requested resource /manager/ec2_action', data={'field': ['validation-error']})) assert status_code == 400 assert response.json == { - 'code': 400, 'message': 'Input validation failed for /manager/ec2_action', + 'code': 400, 'message': 'Input validation failed for requested resource /manager/ec2_action', 'validation_errors': {'field': ['validation-error']} } diff --git a/api/validation/__init__.py b/api/validation/__init__.py index e2639043d..ab9e98d38 100644 --- a/api/validation/__init__.py +++ b/api/validation/__init__.py @@ -30,7 +30,7 @@ def wrapper(func): def decorated(*pargs, **kwargs): errors = __validate_request(request, body_schema=body, params_schema=params, cookies_schema=cookies, raise_on_missing_body=raise_on_missing_body) if errors: - raise ValidationError(f'Input validation failed for {request.path}', data=errors) + raise ValidationError(f'Input validation failed for requested resource {request.path}', data=errors) return func(*pargs, **kwargs) return decorated