fix security issues 80 and 81; comments; whitespace; docs; tag for re…

…lease

NEWS.md

@@ -1,21 +1,52 @@
-XML-RPC for PHP version xxx - unreleased
+XML-RPC for PHP version 4.9 - 2022/11/28
 
 * security fix: hardened the `Client::send()` method against misuse of the `$method` argument (issue #81).
   Abusing its value, it was possible to force the client to _access local files_ or _connect to undesired urls_ instead
   of the intended target server's url (the one used in the Client constructor).
 
-  This weakness only affects installations where all of the following conditions apply at the same time:
+  This weakness only affects installations where all the following conditions apply, at the same time:
 
   - the xmlrpc Client is used, ie. not xmlrpc servers
   - untrusted data (eg. data from remote users) is used as value for the `$method` argument of method `Client::send()`,
     in conjunction with conditions which trigger usage of curl as http transport (ie. either using the https, http11 or
     http2 protocols, or calling `Client::setUseCurl()` beforehand)
-  - make the resulting Response's object `httpResponse` member, which is intended to be used for debugging purposes only,
-    available to 3rd parties, eg. by displaying it to the end user or serializing it in some storage (note that the
-    same data can also be accessed via magic property `Response::raw_data`, and in the Request's `httpResponse` member)
+  - either have set the Clients `return_type` property to 'xml', or make the resulting Response's object `httpResponse`
+    member, which is intended to be used for debugging purposes only, available to 3rd parties, eg. by displaying it to
+    the end user or serializing it in some storage (note that the same data can also be accessed via magic property
+    `Response::raw_data`, and in the Request's `httpResponse` member)
 
   This is most likely a very uncommon usage scenario, and as such the severity of this issue can be considered low.
 
+  If it is not possible to upgrade to this release of the library at this time, a proactive security measure, to avoid
+  the Client accessing any local file on the server which hosts it, is to add the following call to your code:
+
+      $client->setCurlOptions([CURLOPT_PROTOCOLS, CURLPROTO_HTTPS|CURLPROTO_HTTP]);
+
+* security fix: hardened the `Wrapper::buildClientWrapperCode` method's code generation against _code injection_ via
+  usage of a malevolent `$client` argument (issue #80).
+
+  In order for this weakness to be exploited, the following conditions have to apply, at the same time:
+
+  - method `Wrapper::buildClientWrapperCode`, or any methods which depend on it, such as `Wrapper::wrapXmlrpcServer`,
+    `Wrapper::wrapXmlrpcMethod` or `Wrapper::buildWrapMethodSource` must be in use. Note that they are _not_ used by
+    default in either the Client or Server classes provided by the library; the developer has to specifically make use
+    of them in his/her own code
+  - the `$client` argument to either of those methods should have been built with malicious data, ie. data controlled
+    by a 3rd party, passed to its constructor call
+
+  This is most likely an uncommon usage scenario, and as such the severity of this issue can be considered low.
+
+  *NB* the graphical debugger which is shipped as part of the library is vulnerable to this, when used with the option
+  "Generate stub for method call" selected. In that case, the debugger will _display_ but not _execute_ the
+  malicious code, which would have to be provided via carefully crafted values for the "Address" and "Path" inputs.
+
+  The attack scenario in this case is that a developer copies into his/her own source code the php snippet generated
+  by the debugger, in a situation where the debugger is used with "Address"/"Path" input values supplied by a 3rd party.
+  The malicious payload in the "Address"/"Path" input values should be easily recognized as suspicious by any barely
+  proficient developer, as it resembles a bog-standard injection attack.
+  It goes without saying that a responsible developer should not blindly copy and paste into his/her own code anything
+  generated by a 3rd party tool, such as the phpxmlrpc debugger, without giving it at least a cursory scan.
+
 * fixed: a php warning on php 8 when parsing responses which do not have a Content-Type header (issue #104)
 
 * fixed: added a missing html-escaping call in demo file `introspect.php`
@@ -26,10 +57,17 @@ XML-RPC for PHP version xxx - unreleased
 
 * fixed: use of uninitialized var when accessing nonexisting member of legacy class `xmlrpc_server` - thanks SonarQube
 
+* new: the Client class now supports making calls which follow http redirections (issue #77). For that to work, use this code:
+
+      $client->setUseCurl(\PhpXmlRpc\Client::USE_CURL_ALWAYS);
+      $client->setCurlOptions([CURLOPT_FOLLOWLOCATION => true, CURLOPT_POSTREDIR => 3]);
+
 * new: allow users of the library to get more fine-grained information about errors in parsing received responses by
   overriding the integer value of `PhpXmlRpc::$xmlrpcerr['invalid_xml']`, `PhpXmlRpc::$xmlrpcerr['xml_not_compliant']`,
   `PhpXmlRpc::$xmlrpcerr['xml_parsing_error']` and the equivalent `PhpXmlRpc::$xmlrpcstr` strings (feature req. #101)
 
+* improved: added the HTTP/2 protocol to the debugger
+
 * improved: CI tests now run on php versions 5.4 and 5.5, besides all more recent ones
 
 * improved: the test container for local testing now defaults to php 7.4 on ubuntu 20 focal
@@ -93,6 +131,7 @@ XML-RPC for PHP version 4.6.0 - 2021/12/9
 * new: increase flexibility in class composition by adopting a Dependency Injection (...ish) pattern:
   it is now possible to swap out the Logger, XMLParser and Charset classes with similar ones of your own making.
   Example code:
+
       // 1. create an instance of a custom character encoder
       // $myCharsetEncoder = ...
       // 2. then use it while serializing a Request:
@@ -228,7 +267,7 @@ XML-RPC for PHP version 4.3.0 - 2017/11/6
 * improved: added unit tests for Basic and Digest http auth. Also improved tests suite
 
 * new: allow to force usage of curl for http 1.0 calls, as well as plain socket for https calls, via the method
-    `Client::setUseCurl()`
+  `Client::setUseCurl()`
 
 
 XML-RPC for PHP version 4.2.2 - 2017/10/15
@@ -249,17 +288,17 @@ XML-RPC for PHP version 4.2.0 - 2017/6/30
 XML-RPC for PHP version 4.1.1 - 2016/10/1
 
 * fixed: error in server class: undefined function php_xmlrpc_encode (only triggered when not using the compatibility
-    shim with old versions)
+  shim with old versions)
 
 
 XML-RPC for PHP version 4.1.0 - 2016/6/26
 
 * improved: Added support for receiving <I8> and <EX:I8> integers, sending <I8>
 
-    If php is compiled in 32 bit mode, and an i8 int is received from a 3rd party, and error will be emitted.
-    Integers sent from the library to 3rd parties can be encoded using the i8 tag, but default to using 'int' by default;
-    the developer will have to create values as i8 explicitly if needed.
-    The library does *not* check if an outgoing integer is too big to fit in 4 bytes and convert it to an i8 automatically.
+  If php is compiled in 32 bit mode, and an i8 int is received from a 3rd party, and error will be emitted.
+  Integers sent from the library to 3rd parties can be encoded using the i8 tag, but default to using 'int' by default;
+  the developer will have to create values as i8 explicitly if needed.
+  The library does *not* check if an outgoing integer is too big to fit in 4 bytes and convert it to an i8 automatically.
 
 
 XML-RPC for PHP version 4.0.1 - 2016/3/27
@@ -358,14 +397,15 @@ PLEASE READ CAREFULLY THE NOTES BELOW to insure a smooth upgrade.
   previously those messages would just disappear (this is visible e.g. in the debugger)
 
 * changed: debug info handling
+
     - at debug level 1, the rebuilt php objects are not dumped to screen (server-side already did that)
     - at debug level 1, curl communication info are not dumped to screen
     - at debug level 1, the tests echo payloads of failures; at debug level 2 all payloads
 
 * improved: makefiles have been replaced with a php_based pakefile
 
 * improved: the source for the manual is stored in asciidoc format, which can be displayed natively by GitHub
-  with nice html formatting. Also the HTML version generated by hand and bundled in tarballs is much nicer
+  with nice html formatting. Also, the HTML version generated by hand and bundled in tarballs is much nicer
   to look at than previous versions
 
 * improved: all php code is now formatted according to the PSR-2 standard
@@ -402,7 +442,7 @@ the library is still considered to be production quality.
 * improved: add option 'dates_as_objects' to php_xmlrpc_decode to return dateTime objects for xmlrpc datetimes
 * improved: add new method SetCurlOptions to xmrlpc_client to allow extra flexibility in tweaking http config, such as
   explicitly binding to an ip address
-* improved: add new method SetUserAgent to xmrlpc_client to to allow having different user-agent http headers
+* improved: add new method SetUserAgent to xmrlpc_client to allow having different user-agent http headers
 * improved: add a new member variable in server class to allow fine-tuning of the encoding of returned values when the
   server is in 'phpvals' mode
 * improved: allow servers in 'xmlrpcvals' mode to also register plain php functions by defining them in the dispatch map
@@ -413,20 +453,19 @@ the library is still considered to be production quality.
 
 XML-RPC for PHP version 2.2.2 - 2009/03/16
 
-This release corrects all bugs that have been reported and sucesfully reproduced since
+This release corrects all bugs that have been reported and successfully reproduced since
 version 2.2.1.
 Regardless of the intimidating message about dropping PHP 4 support, it still does
 support that ancient, broken and insecure platform.
 
-
 * fixed: php warning when receiving 'false' in a bool value
 * fixed: improve robustness of the debugger when parsing weird results from non-compliant servers
 * fixed: format floating point values using the correct decimal separator even when php locale is set to one that uses
   comma
 * fixed: use feof() to test if socket connections are to be closed instead of the number of bytes read (rare bug when
   communicating with some servers)
 * fixed: be more tolerant in detection of charset in http headers
-* fixed: fix encoding of UTF8 chars outside of the BMP plane
+* fixed: fix encoding of UTF8 chars outside the BMP plane
 * fixed: fix detection of zlib.output_compression
 * improved: allow the add_to_map server method to add docs for single params too
 * improved: added the possibility to wrap for exposure as xmlrpc methods plain php class methods, object methods and
@@ -435,7 +474,7 @@ support that ancient, broken and insecure platform.
 
 XML-RPC for PHP version 2.2.1 - 2008/03/06
 
-This release corrects all bugs that have been reported and sucesfully reproduced.
+This release corrects all bugs that have been reported and successfully reproduced.
 It is the last release of the library that will support PHP 4.
 
 * fixed: work around bug in php 5.2.2 which broke support of HTTP_RAW_POST_DATA
@@ -487,7 +526,7 @@ optionally reenabled).
 
 The constructor of xmlrpcval() values has seen major changes, and it will not
 throw a php warning anymore when invoked using an unknown xmlrpc type: the
-error will only be written to php error log. Also new xmlrpcval('true', 'boolean')
+error will only be written to php error log. Also, new xmlrpcval('true', 'boolean')
 is not supported anymore.
 
 MAJOR IMPROVEMENTS:

README.md

@@ -14,7 +14,7 @@ Documentation
 See the documentation page at [gggeek.github.io/phpxmlrpc](https://gggeek.github.io/phpxmlrpc) for a list of the
 library main features and all project related information.
 
-The user manual can be found in the doc/manual directory, in Asciidoc format: [phpxmlrpc_manual.adoc](doc/manual/phpxmlrpc_manual.adoc)
+The user manual can be found in the doc/manual directory, in AsciiDoc format: [phpxmlrpc_manual.adoc](doc/manual/phpxmlrpc_manual.adoc)
 
 Older release tarballs also contain HTML and PDF versions of the manual, as well as an automatically generated API documentation.
 

src/Client.php

@@ -103,7 +103,7 @@ class Client
      * Decides the content of Response objects returned by calls to send() and multicall().
      * Valid values are 'xmlrpcvals', 'phpvals' or 'xml'.
      *
-     * Determines whether the value returned inside an Response object as results of calls to the send() and multicall()
+     * Determines whether the value returned inside a Response object as results of calls to the send() and multicall()
      * methods will be a Value object, a plain php value or a raw xml string.
      * Allowed values are 'xmlrpcvals' (the default), 'phpvals' and 'xml'.
      * To allow the user to differentiate between a correct and a faulty response, fault responses will be returned as
@@ -669,7 +669,7 @@ protected function sendPayloadSocket($req, $server, $port, $timeout = 0, $userna
         /// @todo log a warning if passed an unsupported method
 
         if ($port == 0) {
-            $port = ( $method === 'https' ) ? 443 : 80;
+            $port = ($method === 'https') ? 443 : 80;
         }
 
         // Only create the payload if it was not created previously
@@ -728,7 +728,7 @@ protected function sendPayloadSocket($req, $server, $port, $timeout = 0, $userna
         } else {
             $connectServer = $server;
             $connectPort = $port;
-            $transport = ( $method === 'https' ) ? 'tls' : 'tcp';
+            $transport = ($method === 'https') ? 'tls' : 'tcp';
             $uri = $this->path;
         }
 
@@ -912,6 +912,10 @@ protected function sendPayloadCURL($req, $server, $port, $timeout = 0, $username
             $proxyUsername, $proxyPassword, $proxyAuthType, $method, $keepAlive, $key,
             $keyPass, $sslVersion);
 
+        if (!$curl) {
+            return new Response(0, PhpXmlRpc::$xmlrpcerr['curl_fail'], PhpXmlRpc::$xmlrpcstr['curl_fail'] . ': error during curl initialization. Check php error log for details');
+        }
+
         $result = curl_exec($curl);
 
         if ($this->debug > 1) {
@@ -940,10 +944,12 @@ protected function sendPayloadCURL($req, $server, $port, $timeout = 0, $username
                 curl_close($curl);
             }
             $resp = $req->parseResponse($result, true, $this->return_type);
-            // if we got back a 302, we can not reuse the curl handle for later calls
-            if ($resp->faultCode() == PhpXmlRpc::$xmlrpcerr['http_error'] && $keepAlive) {
-                curl_close($curl);
-                $this->xmlrpc_curl_handle = null;
+            if ($keepAlive) {
+                /// @todo if we got back a 302 or 308, we should not reuse the curl handle for later calls
+                if ($resp->faultCode() == PhpXmlRpc::$xmlrpcerr['http_error']) {
+                    curl_close($curl);
+                    $this->xmlrpc_curl_handle = null;
+                }
             }
         }
 
@@ -997,9 +1003,16 @@ protected function prepareCurlHandle($req, $server, $port, $timeout = 0, $userna
                 } else {
                     // http, https
                     $protocol = $method;
+                    if (strpos($protocol, ':') !== false) {
+                        $this->getLogger()->errorLog('XML-RPC: ' . __METHOD__ . ": warning - attempted hacking attempt?. The curl protocol requested for the call is: '$protocol'");
+                        return false;
+                    }
                 }
             }
             $curl = curl_init($protocol . '://' . $server . ':' . $port . $this->path);
+            if (!$curl) {
+                return false;
+            }
             if ($keepAlive) {
                 $this->xmlrpc_curl_handle = $curl;
             }
@@ -1055,7 +1068,7 @@ protected function prepareCurlHandle($req, $server, $port, $timeout = 0, $userna
             curl_setopt($curl, CURLOPT_TIMEOUT, $timeout == 1 ? 1 : $timeout - 1);
         }
 
-        switch($method) {
+        switch ($method) {
             case 'http10':
                 curl_setopt($curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_0);
                 break;
@@ -1166,7 +1179,7 @@ protected function prepareCurlHandle($req, $server, $port, $timeout = 0, $userna
      * @param Request[] $reqs an array of Request objects
      * @param integer $timeout connection timeout (in seconds). See the details in the docs for the send() method
      * @param string $method the http protocol variant to be used. See the details in the docs for the send() method
-     * @param boolean fallback When true, upon receiving an error during multicall, multiple single calls will be
+     * @param boolean $fallback When true, upon receiving an error during multicall, multiple single calls will be
      *                         attempted
      *
      * @return Response[]
@@ -1318,7 +1331,7 @@ private function _try_multicall($reqs, $timeout, $method)
             }
 
             $response = array();
-            foreach($rets as $val) {
+            foreach ($rets as $val) {
                 switch ($val->kindOf()) {
                     case 'array':
                         if ($val->count() != 1) {

src/PhpXmlRpc.php

@@ -92,7 +92,7 @@ class PhpXmlRpc
     public static $xmlrpc_internalencoding = "UTF-8";
 
     public static $xmlrpcName = "XML-RPC for PHP";
-    public static $xmlrpcVersion = "4.8.1";
+    public static $xmlrpcVersion = "4.9.0";
 
     // let user errors start at 800
     public static $xmlrpcerruser = 800;

src/Wrapper.php

@@ -1126,19 +1126,18 @@ public function wrapXmlrpcServer($client, $extraOptions = array())
 
     /**
      * Given necessary info, generate php code that will build a client object just like the given one.
-     * Take care that no full checking of input parameters is done to ensure that
-     * valid php code is emitted.
+     * Take care that no full checking of input parameters is done to ensure that valid php code is emitted.
      * @param Client $client
-     * @param bool $verbatimClientCopy when true, copy all of the state of the client, except for 'debug' and 'return_type'
+     * @param bool $verbatimClientCopy when true, copy the whole state of the client, except for 'debug' and 'return_type'
      * @param string $prefix used for the return_type of the created client
      * @param string $namespace
      *
      * @return string
      */
     protected function buildClientWrapperCode($client, $verbatimClientCopy, $prefix = 'xmlrpc', $namespace = '\\PhpXmlRpc\\')
     {
-        $code = "\$client = new {$namespace}Client('" . str_replace("'", "\'", $client->path) .
-            "', '" . str_replace("'", "\'", $client->server) . "', $client->port);\n";
+        $code = "\$client = new {$namespace}Client('" . str_replace(array("\\", "'"), array("\\\\", "\'"), $client->path) .
+            "', '" . str_replace(array("\\", "'"), array("\\\\", "\'"), $client->server) . "', $client->port);\n";
 
         // copy all client fields to the client that will be generated runtime
         // (this provides for future expansion or subclassing of client obj)