search

MrKampf / proxmoxVE

The is a PHP 8 ProxmoxVe API client. With it you can easily and quickly retrieve information from your Proxmox server or cluster.
GNU General Public License v3.0
51 stars 26 forks source link

DDoS Security Vulnerability #18

Closed ericwang401 closed 1 year ago

ericwang401 commented 2 years ago

Describe the bug When the ProxmoxVE package tries to reach an unresponsive Proxmox server, it never times out.

This could be a valid IP address, but the server on the other end could be down.

When this happens, PHP-FPM is stuck waiting for GuzzleHttp to timeout or return a response, which prevents the web server from answering any other traffic.

This is problematic because a single browser could try to ping an unresponsive Proxmox server and take down an entire web server because the request will never time out.

The root cause of this issue is that get, post, delete, put, and getCSRFToken in Proxmox/Helper/Api.php LACK a timeout option, so GuzzleHttp will wait INDEFINITELY for the Proxmox server to respond.

To Reproduce Steps to reproduce the behavior:

  1. Supply a valid IP address that is unresponsive
  2. Send a request to fetch for server status on a QEMU instance

Expected behavior The request should time out within somewhere between 10-20 seconds.

Screenshots https://i.imgur.com/pnbAAbT.gif

I tried to show laravel.log, but there is not any logs because the requests haven't timed out yet.

Desktop (please complete the following information):

Server (please complete the following information):

Additional context To fix this issue, add a header specifying a timeout.

A STRONGLY recommended way to fix this is allow the developer to configure GuzzleHttp options.

This will let any person who's using this package to set a custom timeout option and also add any other headers they might need.

ericwang401 commented 2 years ago

Also, I recommend removing the try...catch statements in your code. It's not a great idea to catch errors and return null or print the exception. If you return null, the developer has no idea what the error is and if you print the error, the code will continue executing until it hits another exception. This current method will only result in more confusing errors that will require more hours of debugging.

I suggest either removing your try...catch statements or making your own exceptions.

MrKampf commented 2 years ago

Yes, you can do that. I'm working on the library with the direct Laravel HTTP client, so that you can look at the error directly with e.g. Telescope.

You can prevent this by lowering the timeout in PHP, but I'll be happy to add some options so that you can set something like this.

MrKampf commented 2 years ago

If you are interested in testing and viewing the Laravel library just send me a short mail to daniel.engelschalk@gitcom.cloud and I will give you access ;)

ericwang401 commented 2 years ago

Yes, you can do that. I'm working on the library with the direct Laravel HTTP client, so that you can look at the error directly with e.g. Telescope.

You can prevent this by lowering the timeout in PHP, but I'll be happy to add some options so that you can set something like this.

Is it possible if an option can be added? I would appreciate it a lot! Thanks!

ericwang401 commented 2 years ago

If you are interested in testing and viewing the Laravel library just send me a short mail to daniel.engelschalk@gitcom.cloud and I will give you access ;)

I've also sent you an email. Thanks!

ericwang401 commented 1 year ago

Vulnerability is still present. Though I believe it's an intended behavior.