Skip to content

Commit b714e97

Browse files
westey-mwestey-m
authored
.Net: Improve OpenAPI uri validation (#14761)
### Motivation and Context Improve OpenAPI uri validation ### Description Improve OpenAPI uri validation ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [ ] The code builds clean without any errors or warnings - [ ] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄
1 parent df1cb33 commit b714e97

3 files changed

Lines changed: 118 additions & 5 deletions

File tree

‎dotnet/src/Functions/Functions.OpenApi/RestApiOperationServerUrlValidationOptions.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ public class RestApiOperationServerUrlValidationOptions
4242
/// <summary>
4343
/// Gets or sets the explicit allowlist of base URLs. A request whose final URL matches one
4444
/// of these entries (same scheme, host, port, and path prefix) is permitted regardless of
45-
/// the implicit scheme and private-IP gates.
45+
/// the implicit scheme and private-IP gates. Scheme and host are compared case-insensitively;
46+
/// the path is compared case-sensitively. Request URLs whose path contains a <c>.</c> or
47+
/// <c>..</c> segment, including percent-encoded forms such as <c>%2e%2e</c> or <c>..%2f</c>,
48+
/// never match an entry.
4649
/// </summary>
4750
/// <remarks>
4851
/// For example, with <c>AllowedBaseUrls = [new Uri("https://api.example.com/v1")]</c>:

‎dotnet/src/Functions/Functions.OpenApi/ServerUrlValidator.cs‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,14 @@ private static bool TryMatchAllowedBaseUrl(Uri url, IReadOnlyList<Uri>? allowedB
8686
return false;
8787
}
8888

89-
var urlPath = url.GetLeftPart(UriPartial.Path);
89+
// Dot-segments (raw or percent-encoded, e.g. "..%2f") may be resolved by downstream
90+
// servers or proxies and escape the allowed path, so such URLs never match.
91+
if (RestApiOperation.ContainsDotSegment(url.AbsolutePath))
92+
{
93+
return false;
94+
}
95+
96+
var urlPath = NormalizePercentEncodingCase(url.AbsolutePath);
9097

9198
foreach (var baseUrl in allowedBaseUrls)
9299
{
@@ -103,18 +110,48 @@ private static bool TryMatchAllowedBaseUrl(Uri url, IReadOnlyList<Uri>? allowedB
103110
continue;
104111
}
105112

106-
var baseUrlPath = baseUrl.GetLeftPart(UriPartial.Path);
113+
// Paths are case-sensitive (RFC 3986 section 6.2.2.1), so compare them ordinally.
114+
var baseUrlPath = NormalizePercentEncodingCase(baseUrl.AbsolutePath);
107115
var baseUrlWithSlash = baseUrlPath.EndsWith("/", StringComparison.Ordinal)
108116
? baseUrlPath
109117
: baseUrlPath + "/";
110118

111-
if (string.Equals(urlPath, baseUrlPath, StringComparison.OrdinalIgnoreCase) ||
112-
urlPath.StartsWith(baseUrlWithSlash, StringComparison.OrdinalIgnoreCase))
119+
if (string.Equals(urlPath, baseUrlPath, StringComparison.Ordinal) ||
120+
urlPath.StartsWith(baseUrlWithSlash, StringComparison.Ordinal))
113121
{
114122
return true;
115123
}
116124
}
117125

118126
return false;
119127
}
128+
129+
/// <summary>
130+
/// Uppercases the hex digits of percent-encoded triplets (e.g. "%2f" to "%2F") so that
131+
/// equivalent encodings compare equal (RFC 3986 section 6.2.2.1). Nothing is decoded, so
132+
/// reserved characters such as an encoded "/" stay distinct from the literal separator.
133+
/// </summary>
134+
private static string NormalizePercentEncodingCase(string path)
135+
{
136+
if (path.IndexOf('%') < 0)
137+
{
138+
return path;
139+
}
140+
141+
var chars = path.ToCharArray();
142+
for (int i = 0; i + 2 < chars.Length; i++)
143+
{
144+
if (chars[i] == '%' && IsHexDigit(chars[i + 1]) && IsHexDigit(chars[i + 2]))
145+
{
146+
chars[i + 1] = char.ToUpperInvariant(chars[i + 1]);
147+
chars[i + 2] = char.ToUpperInvariant(chars[i + 2]);
148+
i += 2;
149+
}
150+
}
151+
152+
return new string(chars);
153+
}
154+
155+
private static bool IsHexDigit(char c) =>
156+
(c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F');
120157
}

‎dotnet/src/Functions/Functions.UnitTests/OpenApi/ServerUrlValidatorTests.cs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,79 @@ public async Task ItShouldAllowExplicitAllowedBaseUrlEvenForPrivateAddressAsync(
137137
await ServerUrlValidator.ValidateAsync(url, options);
138138
}
139139

140+
[Theory]
141+
[InlineData("https://api.example.com/V1/Public/resource")]
142+
[InlineData("https://api.example.com/v1/PUBLIC")]
143+
public async Task ItShouldRejectAllowedBaseUrlPathWithDifferentCaseAsync(string url)
144+
{
145+
var options = new RestApiOperationServerUrlValidationOptions
146+
{
147+
AllowedBaseUrls = [new Uri("https://api.example.com/v1/public/")]
148+
};
149+
150+
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
151+
ServerUrlValidator.ValidateAsync(new Uri(url), options));
152+
Assert.Contains("does not match", ex.Message, StringComparison.Ordinal);
153+
}
154+
155+
[Fact]
156+
public async Task ItShouldMatchAllowedBaseUrlSchemeAndHostCaseInsensitivelyAsync()
157+
{
158+
var options = new RestApiOperationServerUrlValidationOptions
159+
{
160+
AllowedBaseUrls = [new Uri("https://api.example.com/v1/public/")]
161+
};
162+
163+
// Should not throw.
164+
await ServerUrlValidator.ValidateAsync(new Uri("HTTPS://API.Example.COM/v1/public/resource"), options);
165+
}
166+
167+
[Theory]
168+
[InlineData("https://api.example.com/v1/a%2Fb/", "https://api.example.com/v1/a%2fb/resource")]
169+
[InlineData("https://api.example.com/v1/a%2fb/", "https://api.example.com/v1/a%2Fb/resource")]
170+
public async Task ItShouldMatchPercentEncodedPathsRegardlessOfHexCaseAsync(string allowedBaseUrl, string url)
171+
{
172+
var options = new RestApiOperationServerUrlValidationOptions
173+
{
174+
AllowedBaseUrls = [new Uri(allowedBaseUrl)]
175+
};
176+
177+
// Should not throw.
178+
await ServerUrlValidator.ValidateAsync(new Uri(url), options);
179+
}
180+
181+
[Fact]
182+
public async Task ItShouldStillRejectPathLetterCaseMismatchWithPercentEncodingAsync()
183+
{
184+
var options = new RestApiOperationServerUrlValidationOptions
185+
{
186+
AllowedBaseUrls = [new Uri("https://api.example.com/v1/a%2Fb/")]
187+
};
188+
189+
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
190+
ServerUrlValidator.ValidateAsync(new Uri("https://api.example.com/v1/A%2fb/resource"), options));
191+
Assert.Contains("does not match", ex.Message, StringComparison.Ordinal);
192+
}
193+
194+
[Theory]
195+
[InlineData("https://api.example.com/v1/public/..%2finternal/keys")]
196+
[InlineData("https://api.example.com/v1/public/..%2Finternal/keys")]
197+
[InlineData("https://api.example.com/v1/public/..%5cinternal/keys")]
198+
[InlineData("https://api.example.com/v1/public/%2e%2e%2finternal/keys")]
199+
[InlineData("https://api.example.com/v1/public/%252e%252e%252finternal/keys")]
200+
[InlineData("https://api.example.com/v1/public/sub/%2e%2e/%2e%2e/internal/keys")]
201+
public async Task ItShouldRejectDotSegmentsUnderAllowedBaseUrlAsync(string url)
202+
{
203+
var options = new RestApiOperationServerUrlValidationOptions
204+
{
205+
AllowedBaseUrls = [new Uri("https://api.example.com/v1/public/")]
206+
};
207+
208+
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
209+
ServerUrlValidator.ValidateAsync(new Uri(url), options));
210+
Assert.Contains("does not match", ex.Message, StringComparison.Ordinal);
211+
}
212+
140213
[Fact]
141214
public async Task ItShouldPreserveExplicitHostnameAllowlistDnsBypassAsync()
142215
{

0 commit comments

Comments
 (0)