diff --git a/dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs b/dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs index 5bfa5a8a1ff2..4f4cd2a48948 100644 --- a/dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs +++ b/dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs @@ -412,19 +412,39 @@ value is string { } strValue && }; /// - /// Validates that the path does not contain dot-segments (. or ..) that could enable path traversal. + /// Validates that the path does not contain dot-segments (. or ..) that could enable path traversal, + /// including percent-encoded forms (e.g. "%2e%2e") that canonicalizes at request time. /// ".." navigates up one path segment, enabling traversal to unintended endpoints. /// "." refers to the current directory — harmless but unexpected, so rejected to prevent misuse. /// /// The path to validate. private static void ValidatePathSegments(string path) { - var segments = path.Split('/'); - for (int i = 0; i < segments.Length; i++) + // Split on the structural path separator first. + foreach (var rawSegment in path.Split('/')) { - if (segments[i] == "." || segments[i] == "..") + // Decode percent-encoding until stable to catch encoded ("%2e") and + // double-encoded ("%252e") dot-segments before URI canonicalization. + var decoded = rawSegment; + for (int i = 0; i < 5; i++) { - throw new KernelException($"Path '{path}' contains a dot-segment, which could lead to path traversal."); + var unescaped = Uri.UnescapeDataString(decoded); + if (string.Equals(unescaped, decoded, StringComparison.Ordinal)) + { + break; + } + + decoded = unescaped; + } + + // A decoded segment may itself contain encoded separators ("%2f"/"%5c"), + // so re-split on both '/' and '\' and reject any resulting dot-segment. + foreach (var segment in decoded.Split('/', '\\')) + { + if (segment == "." || segment == "..") + { + throw new KernelException($"Path '{path}' contains a dot-segment, which could lead to path traversal."); + } } } } diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationTests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationTests.cs index 9b17ae442731..6273c80d494e 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationTests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationTests.cs @@ -1467,6 +1467,92 @@ public void ItShouldAllowDotsInNonSegmentPathParameterValues() Assert.Equal("https://example.com/api/files/report.v2.txt", url.OriginalString); } + [Theory] + [InlineData("/resources/%2e%2e/admin")] + [InlineData("/resources/%2E%2E/admin")] + [InlineData("/resources/%2e./admin")] + [InlineData("/resources/.%2e/admin")] + [InlineData("/resources/%2e%2e%2fadmin")] + [InlineData("/resources/%252e%252e/admin")] + [InlineData("/resources/%2e/admin")] + public void ItShouldRejectEncodedDotSegmentInPathTemplate(string path) + { + // Arrange — operation path template contains an encoded dot-segment that + // System.Uri would canonicalize into a path-traversal at request time. + var sut = new RestApiOperation( + id: "fake_id", + servers: [new RestApiServer("https://example.com/api")], + path: path, + method: HttpMethod.Get, + description: "fake_description", + parameters: [], + responses: new Dictionary(), + securityRequirements: [] + ); + + var arguments = new Dictionary(); + + // Act & Assert — encoded dot-segments must be rejected before URL is built + var ex = Assert.Throws(() => sut.BuildOperationUrl(arguments)); + Assert.Contains("dot-segment", ex.Message); + } + + [Fact] + public void ItShouldRejectEncodedDotSegmentInPathParameter() + { + // Arrange — path parameter value is an encoded ".." (%2e%2e) + var parameters = new List { + new( + name: "id", + type: "string", + isRequired: true, + expand: false, + location: RestApiParameterLocation.Path, + style: RestApiParameterStyle.Simple) + }; + + var sut = new RestApiOperation( + id: "fake_id", + servers: [new RestApiServer("https://example.com/api")], + path: "/resources/{id}/details", + method: HttpMethod.Get, + description: "fake_description", + parameters: parameters, + responses: new Dictionary(), + securityRequirements: [] + ); + + var arguments = new Dictionary { { "id", "%2e%2e" } }; + + // Act & Assert — encoded dot-segments in parameter values must be rejected + var ex = Assert.Throws(() => sut.BuildOperationUrl(arguments)); + Assert.Contains("dot-segment", ex.Message); + } + + [Fact] + public void ItShouldAllowEncodedNonDotSegmentCharactersInPathTemplate() + { + // Arrange — path contains encoded characters that are NOT dot-segments + var sut = new RestApiOperation( + id: "fake_id", + servers: [new RestApiServer("https://example.com/api")], + path: "/resources/a%20b/details", + method: HttpMethod.Get, + description: "fake_description", + parameters: [], + responses: new Dictionary(), + securityRequirements: [] + ); + + var arguments = new Dictionary(); + + // Act + var url = sut.BuildOperationUrl(arguments); + + // Assert — legitimate encoded characters must not be rejected + Assert.Equal("https://example.com/api/resources/a%20b/details", url.OriginalString); + } + [Fact] public void ItShouldEncodeServerVariableValuesLookedUpByArgumentName() { diff --git a/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py b/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py index 570a4352892c..52e1c8aac59f 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py +++ b/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py @@ -2,7 +2,7 @@ import re from typing import Any, Final -from urllib.parse import ParseResult, ParseResultBytes, quote, urlencode, urljoin, urlparse, urlunparse +from urllib.parse import ParseResult, ParseResultBytes, quote, unquote, urlencode, urljoin, urlparse, urlunparse from semantic_kernel.connectors.openapi_plugin.models.rest_api_expected_response import ( RestApiExpectedResponse, @@ -289,8 +289,31 @@ def build_path(self, path_template: str, arguments: dict[str, Any]) -> str: ) continue path_template = path_template.replace(f"{{{parameter.name}}}", quote(str(argument), safe="")) + self._validate_path_segments(path_template) return path_template + @staticmethod + def _validate_path_segments(path: str) -> None: + """Reject dot-segments (. or ..), including percent-encoded forms, that enable path traversal. + + The operation is selected using the raw path but the request URL is built from a canonicalized + path, so encoded dot-segments such as "%2e%2e" must be rejected before the URL is constructed. + """ + for segment in path.split("/"): + decoded = segment + for _ in range(5): + unescaped = unquote(decoded) + if unescaped == decoded: + break + decoded = unescaped + # A decoded segment may contain encoded separators ("%2f"/"%5c"), so re-split on + # both "/" and "\" and reject any resulting dot-segment. + for part in decoded.replace("\\", "/").split("/"): + if part in (".", ".."): + raise FunctionExecutionException( + f"Path '{path}' contains a dot-segment, which could lead to path traversal." + ) + def build_query_string(self, arguments: dict[str, Any]) -> str: """Build the query string for the operation.""" segments = [] diff --git a/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py b/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py index 2dd2488fc506..c59be28e9c55 100644 --- a/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py +++ b/python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py @@ -435,9 +435,9 @@ def test_build_path_prevents_path_traversal(): id="test", method="GET", servers=["https://example.com/"], path="/resource/{id}", params=parameters ) arguments = {"id": "../../admin"} - result = operation.build_path(operation.path, arguments) - # The slashes must be encoded so ../../admin becomes a single path segment, not a traversal - assert result == "/resource/..%2F..%2Fadmin" + # Encoded separators that decode into dot-segments must be rejected, not silently encoded + with pytest.raises(FunctionExecutionException, match="dot-segment"): + operation.build_path(operation.path, arguments) def test_build_path_double_encodes_pre_encoded_values(): @@ -462,6 +462,40 @@ def test_build_path_encodes_unicode_characters(): assert result == "/resource/caf%C3%A9%20r%C3%A9sum%C3%A9" +@pytest.mark.parametrize( + "path", + [ + "/resources/../admin", + "/resources/./admin", + "/resources/%2e%2e/admin", + "/resources/%2E%2E/admin", + "/resources/%2e/admin", + "/resources/%2e%2e%2fadmin", + "/resources/%252e%252e/admin", + ], +) +def test_build_path_rejects_dot_segment_in_template(path): + operation = RestApiOperation(id="test", method="GET", servers=["https://example.com/"], path=path, params=[]) + with pytest.raises(FunctionExecutionException, match="dot-segment"): + operation.build_path(operation.path, {}) + + +def test_build_path_rejects_dot_segment_via_parameter(): + parameters = [RestApiParameter(name="id", type="string", location=RestApiParameterLocation.PATH, is_required=True)] + operation = RestApiOperation( + id="test", method="GET", servers=["https://example.com/"], path="/resource/{id}/details", params=parameters + ) + with pytest.raises(FunctionExecutionException, match="dot-segment"): + operation.build_path(operation.path, {"id": ".."}) + + +def test_build_path_allows_encoded_non_dot_segment_characters(): + operation = RestApiOperation( + id="test", method="GET", servers=["https://example.com/"], path="/resources/a%20b/details", params=[] + ) + assert operation.build_path(operation.path, {}) == "/resources/a%20b/details" + + def test_build_query_string_with_required_parameter(): parameters = [ RestApiParameter(name="query", type="string", location=RestApiParameterLocation.QUERY, is_required=True)