[CXF-7986] WebClient.path method overwrites base URL when given unencoded URL as input

• Type: Bug
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Affects Version/s: 3.3.0
• Component/s: JAX-RS
• Estimated Complexity: Unknown
• Assignee: Andriy Redko
• Reporter: Jim Griswold
• Created: 01/Mar/19 20:48
• Updated: 22/Mar/19 18:35

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 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 to support a test case wherein UriBuilder.fromPath("http://localhost:8080") is expected to initialize the UriBuilder's scheme, host, and port. I don't know whether this is a valid expectation...the Jersey UriBuilder implementation appends that raw value as a path segment and does not set host/scheme/port. I don't see anything in JSR-311 that suggests the path should be parsed to set other components of the URL when a path is used to initialize the UriBuilder. This behavior appears to have been added by commit 3ba70b45de658b98c96485f10c9f688332d7510c as part of CXF-5007.

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
  }
}