Harden IsSafeFileReference against NUL, deep-encoding, and Unicode homoglyph traversal#7913
Merged
Merged
Conversation
IsSafeFileReference validated the raw file reference, so percent-encoded traversal payloads (e.g. %2e%2e, %2f, %3a) and double-encoded variants bypassed the absolute-URI, rooted-path, drive-path and '..'-segment checks and were treated as safe. This allowed a malicious static_template.file to resolve outside the manifest package (CWE-22/CWE-829). Percent-decode the reference in a bounded loop until stable before validating, defeating single and multi-level encoding. Adds regression tests for the reported payloads plus mixed-case, encoded-backslash, encoded-URI and double-encoded variants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…trol chars, and Unicode homoglyphs Security audit of the percent-encoding path-traversal fix surfaced three residual bypasses in IsSafeFileReference: (1) an embedded NUL from %00 was accepted and defeated the '..' segment check; (2) traversal encoded deeper than the decode budget left %XX residue that was accepted (fail-open); (3) Unicode full-width homoglyphs (e.g. %EF%BC%8E) were not treated as '..'. Fail closed when percent-encoding remains after the decode budget, reject control characters, and fold Unicode compatibility forms (NFKC) before validation. Adds regression tests for each vector. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…e-file-reference-encoding # Conflicts: # src/Kiota.Builder/OpenApiExtensions/OpenApiAiCapabilitiesExtension.cs # tests/Kiota.Builder.Tests/OpenApiExtensions/OpenApiAiCapabilitiesExtensionTests.cs
Addresses PR review feedback: replace the manual foreach filter loop with a predicate-based Any check for clearer intent. Behavior unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Code Coverage OverviewLanguages: C# C# / code-coverage/dotnetThe overall coverage in the Show a code coverage summary of the most covered files.
Code Coverage is in Public Preview. Learn more and provide us with your feedback. |
gavinbarron
enabled auto-merge (squash)
July 7, 2026 22:24
ramsessanchez
approved these changes
Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7912
Summary
Security-audit follow-up to #7910. Auditing the percent-encoding path-traversal fix against a broader payload matrix surfaced three residual bypasses in
ExtensionResponseSemanticsStaticTemplate.IsSafeFileReference, all confirmed by exercising the real method:%00) —card%00.jsonandsafe.json%00%2e%2e%2fetc%2fpasswdwere accepted. The NUL was not rejected and, when fused with the preceding path segment, defeated the exact-match..segment check.%25252525252e..., 6×+) left%XXresidue that was accepted verbatim; a consumer decoding further resolves the traversal.%EF%BC%8E%EF%BC%8E/card.json(→．．/card.json) was accepted because full-width．is not literal..Fix
After the bounded percent-decode loop,
IsSafeFileReferencenow:%XXafter the budget) instead of accepting it.Tests
Extended the
StaticTemplateIsSafeFileReferenceValidatesPaths[Theory]with regression cases for NUL, deep-encoding, and full-width (literal + percent-encoded UTF-8) payloads.dotnet test --filter "FullyQualifiedName~OpenApiAiCapabilitiesExtensionTest"→ 38 passed, 0 failed.Out of scope (audited, not vulnerable)
AllowedExternalOriginsStreamLoader(#7888): path allow-entries canonicalize encoded..before matching, so percent-encoding does not bypass them.