Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding extra slashes after domain name in url allows you to bypass JWT Authentication #50

Closed
gilz688 opened this issue Feb 27, 2017 · 6 comments

Comments

@gilz688
Copy link

gilz688 commented Feb 27, 2017

The following array was passed to JwtAuthentication class constructor:

$options = [
    "secret" => $_ENV["JWT_SECRET"],
    "path" => ["/api/v1"],
    "passthrough" => ["/api/v1/login"]
]

Sending an HTTP request to "http://localhost/api/v1/restricted" returns HTTP 401 Unauthorized but for some reason I am able to bypass JWT authentication by adding one or more extra slashes after the domain name. e.g. "http://localhost//api/v1/restricted"

@tuupola
Copy link
Owner

tuupola commented Feb 27, 2017

Add a logger or error handler to debug why the the authentication fails.

https://github.com/tuupola/slim-jwt-auth#logger
https://github.com/tuupola/slim-jwt-auth#error

@gilz688
Copy link
Author

gilz688 commented Feb 27, 2017

HTTP 401 Unauthorized is expected because I made the request without the token in the Authorization header. (I forgot to mention it in the first comment.)

@tuupola
Copy link
Owner

tuupola commented Feb 27, 2017

Ok I misunderstood. It seems that even though /foo and ///foo are different URL's for browser, on server side it seems to map to same url. A bit surprising behaviour but quickly checking RFC 3986 this seems to be correct behaviour.

Will commit a fix soon.

@gilz688
Copy link
Author

gilz688 commented Feb 27, 2017

While waiting for your commit, I was able to temporarily fix the issue by replacing:
$uri = str_replace("//", "/", $uri); with $uri = preg_replace("/(\/)+/", "/", $uri); in RequestPathRule.php line 52.

@tuupola
Copy link
Owner

tuupola commented Feb 27, 2017

Yep, this is what I did: $uri = preg_replace("#/+#", "/", $uri);.

tuupola added a commit that referenced this issue Feb 27, 2017
RequestPathRule now removes multiple slashes from the URI before
determining whether the path should be authenticated or not. For client
/foo and //foo are different URIs. On server side it depends on
implementation but they usually map to the same route action.

Different PSR-7 implementations were behaving in different way. Diactoros
removes multiple leading slashes while Slim does not.

    * If you are authenticating a subfolder, for example /api, with Slim
      was possible to bypass authentication by doing a request to //api.

    * If you are using default setting of authenticating all routes you
      were not affected.

    * Diactoros was not affected.

Fixes bug #50.
If you were using default setting of authenticating all routes you were not affected.
tuupola added a commit that referenced this issue Feb 27, 2017
This currently breaks encrypted cookie tests and they are disabled.

Fixes #50 for 1.x branch.
tuupola added a commit to tuupola/slim-basic-auth that referenced this issue Feb 27, 2017
RequestPathRule now removes multiple slashes from the URI before
determining whether the path should be authenticated or not. For
client /foo and //foo are different URIs. On server side it
depends on implementation but they usually map to the same route
action.

Different PSR-7 implementations were behaving in different way.
Diactoros removes multiple leading slashes while Slim does not.

    * If you are authenticating a subfolder, for example /api,
      with Slim was possible to bypass authentication by doing
      a request to //api.

    * If you are using default setting of authenticating all
      routes you were not affected.

    * Diactoros was not affected.

See tuupola/slim-jwt-auth#50
tuupola added a commit to tuupola/slim-basic-auth that referenced this issue Feb 27, 2017
@tuupola
Copy link
Owner

tuupola commented Feb 27, 2017

Fixed in 2.3.2 and 1.0.4.

@tuupola tuupola closed this as completed Feb 27, 2017
tuupola added a commit that referenced this issue Apr 28, 2017
RequestPathRule now removes multiple slashes from the URI before
determining whether the path should be authenticated or not. For client
/foo and //foo are different URIs. On server side it depends on
implementation but they usually map to the same route action.

Different PSR-7 implementations were behaving in different way. Diactoros
removes multiple leading slashes while Slim does not.

    * If you are authenticating a subfolder, for example /api, with Slim
      was possible to bypass authentication by doing a request to //api.

    * If you are using default setting of authenticating all routes you
      were not affected.

    * Diactoros was not affected.

Fixes bug #50.
If you were using default setting of authenticating all routes you were not affected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants