pve2-api-php-client icon indicating copy to clipboard operation
pve2-api-php-client copied to clipboard

[Security] cURL SSL/TLS Verification -> option; Verbose Logging -> option/disabled

Open Anuril opened this issue 9 months ago • 2 comments

I found several security flaws in the action function.

SSL/TLS Verification Disabled

https://github.com/CpuID/pve2-api-php-client/blob/b9ef9cb4f040cf71bfa5281b1e31bfae7a984474/pve2_api.class.php#L219C20-L219C20

curl_setopt($prox_ch, CURLOPT_SSL_VERIFYPEER, false);
curl_setopt($prox_ch, CURLOPT_SSL_VERIFYHOST, false);

This makes the requests vulnerable to Man-in-the-Middle (MITM) attacks, where an attacker can intercept and potentially alter the communication between the client and the server.

Potential Information Disclosure & Lack of Data Sanitization

Also described in #45 The function logs extensive details about the responses it receives, including headers and potentially sensitive data. If the logs are improperly secured or misconfigured, it could lead to information disclosure. Below is the code segment responsible for this:

https://github.com/CpuID/pve2-api-php-client/blob/b9ef9cb4f040cf71bfa5281b1e31bfae7a984474/pve2_api.class.php#L233

error_log("----------------------------------------------\n" .
  "FULL RESPONSE:\n\n{$action_response}\n\nEND FULL RESPONSE\n\n" .
  "Headers:\n\n{$header_response}\n\nEnd Headers\n\n" .
  "Data:\n\n{$body_response}\n\nEnd Data\n\n" .
  "RESPONSE ARRAY:\n\n{$action_response_export}\n\nEND RESPONSE ARRAY\n" .
  "----------------------------------------------");

The data received from the API response in the action function is directly logged and used without any visible sanitization or validation, making it potentially susceptible to security vulnerabilities, especially if this data is used elsewhere in the application.

Potential Inconsistent Return Types

For “PUT” HTTP method, it returns a Boolean true if the HTTP response is 200, but for other HTTP methods, it returns the ‘data’ part of the response. This inconsistency might lead to bugs or unexpected behavior in the parts of the application that use this function, indirectly causing security flaws.

In addition to the observed flaws, manually parsing HTTP responses, as seen in this function, is not a best practice, as it is error-prone and can lead to security vulnerabilities; it is generally safer and more efficient to use well-established libraries or built-in functions for processing HTTP responses, which are designed to handle various edge cases and anomalies in a secure manner.

Anuril avatar Sep 28 '23 19:09 Anuril