lhotari commented on code in PR #25628:
URL: https://github.com/apache/pulsar/pull/25628#discussion_r3170395008
##########
pulsar-package-management/core/src/main/java/org/apache/pulsar/packages/management/core/common/PackageName.java:
##########
@@ -136,7 +137,13 @@ public String toString() {
}
public String toRestPath() {
- return String.format("%s/%s/%s/%s/%s", type, tenant, namespace, name,
version);
+ // Use Guava's urlPathSegmentEscaper to safely encode each segment and
prevents Path Traversal (CWE-22)
+ return String.format("%s/%s/%s/%s/%s",
+ type.toString(),
+ UrlEscapers.urlPathSegmentEscaper().escape(tenant),
+ UrlEscapers.urlPathSegmentEscaper().escape(namespace),
+ UrlEscapers.urlPathSegmentEscaper().escape(name),
+ UrlEscapers.urlPathSegmentEscaper().escape(version));
Review Comment:
Since `UrlEscapers.urlPathSegmentEscaper()` is repeated, it should be placed
in a variable, for example
`var escaper = UrlEscapers.urlPathSegmentEscaper()` and then used for each
invocation of `escape`.
##########
pulsar-package-management/core/src/test/java/org/apache/pulsar/packages/management/core/common/PackageNameTest.java:
##########
@@ -116,4 +116,22 @@ public void testPackageNameErrors() {
PackageName name = PackageName.get("function://public/default/test");
Assert.assertEquals("function://public/default/test@latest",
name.toString());
}
+
+ @Test
+ public void testPathTraversalBypassConstructor() throws Exception {
+ // Create a normal, valid package to bypass the splitter
+ PackageName packageName =
PackageName.get("function://tenant-a/ns/name@v1");
+
+ java.lang.reflect.Field tenantField =
PackageName.class.getDeclaredField("tenant");
+ tenantField.setAccessible(true);
+ tenantField.set(packageName, "tenant-a/../../system-tenant");
Review Comment:
Reflection should be avoided in tests, please check
https://lists.apache.org/thread/7gr04sqmzyttx4ln6ydtp3qv0xgo1o6m.
Instead, package private methods should be used annotationed with
`@VisibleForTesting`.
It seems that it's not possible to exploit the "vulnerability" since
PackageName.get already handles validation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
