[
https://issues.apache.org/jira/browse/CXF-7986?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jim Griswold updated CXF-7986:
------------------------------
Description:
When the WebClient.path method is invoked with a URL as an argument, the entire
current URI (not just its path) is overwritten with the supplied value.
Below are example test cases that demonstrate the issue.
I realize that the correct usage here to achieve my desired outcome is to pass
an encoded value to the {{path}} method (e.g. {{http%3A%2F%2Fwww.bar.com}}).
The issue I'm raising here is that the behavior when the input is not
pre-encoded is very surprising and could lead to security vulnerabilities in
applications relying on the WebClient.
Suppose that a developer is making an http request to an external service using
the WebClient, and the developer wants to append a user-supplied value as a
path element using the {{path}} method. If the developer neglected to encode
the input (which seems like a reasonable mistake given that the {{path}} method
encodes other characters aside from {{/}}), a malicious user would be able to
re-route the request to an arbitrary destination.
Further complicating things here (shown by the third test) is that even if you
_do_ encode the path segment with a standard function such as Spring's
{{UriUtils.encodePathSegment}}, the undesired behavior still occurs because
that function does not encode a colon to {{%3A}}. Only when you encode the
colon is the path segment appended as expected. RFC-3986 [appears to
allow|https://tools.ietf.org/html/rfc3986#appendix-A] a colon in a path segment
unencoded.
A safer and more intuitive failure mode would be to only allow the {{path}}
method to append undesired data to the path.
I believe the root of this issue lies in {{UriBuilderImpl}}, which exhibits
similar behavior when its {{path}} method is called. From what I can tell, this
behavior was added by CXF-4281, wherein
{{UriBuilder.fromPath("http://localhost:8080";)}} is expected to initialize the
UriBuilder correctly. One suggestion would be to modify this method to only
allow this usage when a builder is being initialized, and in other cases only
allow {{path}} to append path elements.
{code:java}
import static org.assertj.core.api.Assertions.assertThat;
import org.apache.cxf.jaxrs.client.WebClient;
import org.junit.Test;
public class ClientTest {
@Test
public void pathAllowsAddingURLAsPathElement() {
WebClient webClient = WebClient.create("http://www.foo.com";);
webClient.path("http://www.bar.com";);
assertThat(webClient.getCurrentURI().getHost()).isEqualTo("www.foo.com");
// fails: current uri is "http://www.bar.com";
}
@Test
public void preservesPreviouslyAddedPathParameters() {
WebClient webClient = WebClient.create("http://www.foo.com";);
webClient.path("foo");
webClient.path("bar");
webClient.path("http://www.bar.com";);
assertThat(webClient.getCurrentURI().toString()).startsWith("http://www.foo.com/foo/bar";);
// fails: current uri is "http://www.bar.com";
}
@Test
public void outputOfUriUtilsEncodePathSegmentIsSafe() {
WebClient webClient = WebClient.create("http://www.foo.com";);
webClient.path("http:%2F%2Fwww.bar.com");
assertThat(webClient.getCurrentURI().getHost()).isEqualTo("www.foo.com");
// fails: current uri is http://www.bar.com
}
}{code}
was:
When the WebClient.path method is invoked with a URL as an argument, the entire
current URI (not just its path) is overwritten with the supplied value.
Below are example test cases that demonstrate the issue.
I realize that the correct usage here to achieve my desired outcome is to pass
an encoded value to the {{path}} method (e.g. {{http%3A%2F%2Fwww.bar.com}}).
The issue I'm raising here is that the behavior when the input is not
pre-encoded is very surprising and could lead to security vulnerabilities in
applications relying on the WebClient.
Suppose that a developer is making an http request to an external service using
the WebClient, and the developer wants to append a user-supplied value as a
path element using the {{path}} method. If the developer neglected to encode
the input (which seems like a reasonable mistake given that the {{path}} method
encodes other characters aside from {{/}}), a malicious user would be able to
re-route the request to an arbitrary destination.
Further complicating things here (shown by the third test) is that even if you
_do_ encode the path segment with a standard function such as Spring's
{{UriUtils.encodePathSegment}}, the undesired behavior still occurs because
that function does not encode a colon to {{%3A}}. Only when you encode the
colon is the path segment appended as expected. RFC-3986 [appears to
allow|https://tools.ietf.org/html/rfc3986#appendix-A] a colon in a path segment
unencoded.
A safer and more intuitive failure mode would be to only allow the {{path}}
method to append undesired data to the path.
I believe the root of this issue lies in {{UriBuilderImpl}}, which exhibits
similar behavior when its {{path}} method is called.
{code:java}
import static org.assertj.core.api.Assertions.assertThat;
import org.apache.cxf.jaxrs.client.WebClient;
import org.junit.Test;
public class ClientTest {
@Test
public void pathAllowsAddingURLAsPathElement() {
WebClient webClient = WebClient.create("http://www.foo.com";);
webClient.path("http://www.bar.com";);
assertThat(webClient.getCurrentURI().getHost()).isEqualTo("www.foo.com");
// fails: current uri is "http://www.bar.com";
}
@Test
public void preservesPreviouslyAddedPathParameters() {
WebClient webClient = WebClient.create("http://www.foo.com";);
webClient.path("foo");
webClient.path("bar");
webClient.path("http://www.bar.com";);
assertThat(webClient.getCurrentURI().toString()).startsWith("http://www.foo.com/foo/bar";);
// fails: current uri is "http://www.bar.com";
}
@Test
public void outputOfUriUtilsEncodePathSegmentIsSafe() {
WebClient webClient = WebClient.create("http://www.foo.com";);
webClient.path("http:%2F%2Fwww.bar.com");
assertThat(webClient.getCurrentURI().getHost()).isEqualTo("www.foo.com");
// fails: current uri is http://www.bar.com
}
}{code}
> WebClient.path method overwrites base URL when given unencoded URL as input
> ---------------------------------------------------------------------------
>
> Key: CXF-7986
> URL: https://issues.apache.org/jira/browse/CXF-7986
> Project: CXF
> Issue Type: Bug
> Components: JAX-RS
> Affects Versions: 3.3.0
> Reporter: Jim Griswold
> Priority: Major
>
> When the WebClient.path method is invoked with a URL as an argument, the
> entire current URI (not just its path) is overwritten with the supplied value.
> Below are example test cases that demonstrate the issue.
> I realize that the correct usage here to achieve my desired outcome is to
> pass an encoded value to the {{path}} method (e.g.
> {{http%3A%2F%2Fwww.bar.com}}). The issue I'm raising here is that the
> behavior when the input is not pre-encoded is very surprising and could lead
> to security vulnerabilities in applications relying on the WebClient.
> Suppose that a developer is making an http request to an external service
> using the WebClient, and the developer wants to append a user-supplied value
> as a path element using the {{path}} method. If the developer neglected to
> encode the input (which seems like a reasonable mistake given that the
> {{path}} method encodes other characters aside from {{/}}), a malicious user
> would be able to re-route the request to an arbitrary destination.
> Further complicating things here (shown by the third test) is that even if
> you _do_ encode the path segment with a standard function such as Spring's
> {{UriUtils.encodePathSegment}}, the undesired behavior still occurs because
> that function does not encode a colon to {{%3A}}. Only when you encode the
> colon is the path segment appended as expected. RFC-3986 [appears to
> allow|https://tools.ietf.org/html/rfc3986#appendix-A] a colon in a path
> segment unencoded.
> A safer and more intuitive failure mode would be to only allow the {{path}}
> method to append undesired data to the path.
> I believe the root of this issue lies in {{UriBuilderImpl}}, which exhibits
> similar behavior when its {{path}} method is called. From what I can tell,
> this behavior was added by CXF-4281, wherein
> {{UriBuilder.fromPath("http://localhost:8080";)}} is expected to initialize
> the UriBuilder correctly. One suggestion would be to modify this method to
> only allow this usage when a builder is being initialized, and in other cases
> only allow {{path}} to append path elements.
> {code:java}
> import static org.assertj.core.api.Assertions.assertThat;
> import org.apache.cxf.jaxrs.client.WebClient;
> import org.junit.Test;
> public class ClientTest {
> @Test
> public void pathAllowsAddingURLAsPathElement() {
> WebClient webClient = WebClient.create("http://www.foo.com";);
> webClient.path("http://www.bar.com";);
> assertThat(webClient.getCurrentURI().getHost()).isEqualTo("www.foo.com");
> // fails: current uri is "http://www.bar.com";
> }
> @Test
> public void preservesPreviouslyAddedPathParameters() {
> WebClient webClient = WebClient.create("http://www.foo.com";);
> webClient.path("foo");
> webClient.path("bar");
> webClient.path("http://www.bar.com";);
>
> assertThat(webClient.getCurrentURI().toString()).startsWith("http://www.foo.com/foo/bar";);
> // fails: current uri is "http://www.bar.com";
> }
> @Test
> public void outputOfUriUtilsEncodePathSegmentIsSafe() {
> WebClient webClient = WebClient.create("http://www.foo.com";);
> webClient.path("http:%2F%2Fwww.bar.com");
>
> assertThat(webClient.getCurrentURI().getHost()).isEqualTo("www.foo.com");
> // fails: current uri is http://www.bar.com
> }
> }{code}
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
