[ 
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 two 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.
{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 two 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.

A safer and more intuitive failure mode would be to only allow the {{path}} 
method to append undesired data to the path.
{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";);
 // false: 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 two 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.
> {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)

Reply via email to