Skip to content
This repository has been archived by the owner on Sep 10, 2024. It is now read-only.

Fix directory traversal issue #462

Merged
merged 2 commits into from
Jan 21, 2021

Conversation

kanarip
Copy link
Contributor

@kanarip kanarip commented Jan 11, 2021

This pull request also brings the used terminology of "public_path" in line with Swoole upstream's "document_root".

Reproduce and validate with, repeating ../ as many times as is necessary to get to your / top level directory:

$ curl --path-as-is http://127.0.0.1:1215/js/../etc/hosts

@Arkanius
Copy link
Contributor

Thanks for yout PR!

Well, we could add this at next major version of package, but this is Albert's decision.

I wouldn't go with it for now

@kanarip
Copy link
Contributor Author

kanarip commented Jan 12, 2021

Thanks for yout PR!

Well, we could add this at next major version of package, but this is Albert's decision.

I wouldn't go with it for now

Is it the transition from 'public_path' to 'document_root' standing in the way of accepting the fix to the security issue?

@alecpl
Copy link
Contributor

alecpl commented Jan 12, 2021

"Low priority" for a security issue?

@Arkanius
Copy link
Contributor

Is it the transition from 'public_path' to 'document_root' standing in the way of accepting the fix to the security issue?

yes, that way we had a BC that would force us to release the fix in a major version

@kanarip
Copy link
Contributor Author

kanarip commented Jan 15, 2021

Is it the transition from 'public_path' to 'document_root' standing in the way of accepting the fix to the security issue?

yes, that way we had a BC that would force us to release the fix in a major version

Fair enough, it's now removed.

This should be included in a release as soon as possible, if the proposed solution is indeed acceptable. Thank you.

@Arkanius
Copy link
Contributor

Thanks @kanarip . I'll prepare the release this week. Good job

@Arkanius
Copy link
Contributor

I couldn't reproduce this behavior. When doing a curl to my swoole server Laravel's route handler tried to go to the mentioned route:
image

The json format is my default Route exception handler acting.

Am I missing something?

@kanarip
Copy link
Contributor Author

kanarip commented Jan 18, 2021

I couldn't reproduce this behavior. When doing a curl to my swoole server Laravel's route handler tried to go to the mentioned route:
image

The json format is my default Route exception handler acting.

Am I missing something?

We are definitely not using a custom route exception handler, albeit we are using a fallback route, but another project that doesn't use such a fallback route is used to reproduce below;

Prepare the verification as follows;

  1. Set handle_static_files to true in config/swoole_http.php,
  • Without handle_static_files, this exploit does not work,
  1. Chose a directory that exists in public/, such as js/, or css/, or images/themes/default/,

  2. Append repetitions of ../ as many times as it takes to, from that existing public/js/ directory on your filesystem, get to the top-level / directory of your filesystem, i.e. if you are in /var/www/app/, then public/js/ is /var/www/app/public/js/, meaning you'll need to append ../../../../../,

  • To validate the path you can, cd public/js/ and simply ls -l ../../../../../etc/hosts to see if you have the right amount of ../,
  1. Add a path to an existing file, such as `/etc/hosts';
$ pwd
/home/kanarip/devel/src/projects/laravel.git/src/
$ ls -l public/js/
total 596
-rw-rw-r--. 1 kanarip kanarip 608663  2. Jan 12:57 app.js
$ cd public/js/
$ ls -l ../../../../../../../../../etc/hosts
-rw-r--r--. 1 root root 158 23. Jun 2020  ../../../../../../../../../etc/hosts
$ curl -v --path-as-is http://127.0.0.1:1215/js/../../../../../../../../../etc/hosts
*   Trying 127.0.0.1:1215...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 1215 (#0)
> GET /js/../../../../../../../../../etc/hosts HTTP/1.1
> Host: 127.0.0.1:1215
> User-Agent: curl/7.66.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: application/octet-stream
< Server: swoole-http-server
< Connection: keep-alive
< Date: Mon, 18 Jan 2021 06:45:52 GMT
< Content-Length: 158
< 
127.0.0.1   localhost localhost.localdomain localhost4 localhost4.localdomain4
::1         localhost localhost.localdomain localhost6 localhost6.localdomain6
* Connection #0 to host 127.0.0.1 left intact

@Arkanius
Copy link
Contributor

Oh I see. You're not using nginx in front of swoole, are you?

@kanarip
Copy link
Contributor Author

kanarip commented Jan 18, 2021

Oh I see. You're not using nginx in front of swoole, are you?

No. Neither NGINX nor Apache nor any other "proxying" engine that secretly does more than just proxy.

@Arkanius Arkanius merged commit 41fe378 into swooletw:master Jan 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants