From 7e529d974ee57ff41e78194a8de7318fa423ff8d Mon Sep 17 00:00:00 2001 From: gggeek Date: Thu, 19 Jan 2023 21:01:27 +0000 Subject: [PATCH] mention explicitly the security implications of allowing php exceptions to be displayed as method responses --- composer.json | 2 +- demo/readme.md | 8 ++++++++ demo/server/discuss.php | 5 +++-- demo/server/server.php | 2 +- doc/manual/phpxmlrpc_manual.adoc | 13 ++++++++----- src/Server.php | 8 +++++--- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/composer.json b/composer.json index 478438a2..344e9633 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "ext-curl": "Needed for HTTPS, HTTP2 and HTTP 1.1 support, NTLM Auth etc...", "ext-zlib": "Needed for sending compressed requests and receiving compressed responses, if cURL is not available", "ext-mbstring": "Needed to allow reception of requests/responses in character sets other than ASCII,LATIN-1,UTF-8", - "phpxmlrpc/extras": "Adds more featured Server classes and other useful bits", + "phpxmlrpc/extras": "Adds more featured Server classes, including self-documenting and ajax-enabled servers", "phpxmlrpc/jsonrpc": "Adds support for the JSON-RPC protocol" }, "scripts": { diff --git a/demo/readme.md b/demo/readme.md index 0254314c..f507153f 100644 --- a/demo/readme.md +++ b/demo/readme.md @@ -15,3 +15,11 @@ It goes without saying that the demo files require an installed PHPXMLRPC librar __NB__ These files are meant for _demo_ purposes. They should _not_ be dumped onto a production web server where they are directly accessible by the public at large. We take absolutely _no responsibility_ for any consequences if you do that. + +## More demos + +Please take a look at the demo code in the phpxmlrpc/extras package for more examples of cool Server functionality, +such as having a server generate html documentation for all its xml-rpc methods, act as a reverse proxy, or generate +javascript code to call the xml-rpc methods it exposes. + +See: https://github.com/gggeek/phpxmlrpc-extras/tree/master/demo diff --git a/demo/server/discuss.php b/demo/server/discuss.php index 3288c099..b6bb3af7 100644 --- a/demo/server/discuss.php +++ b/demo/server/discuss.php @@ -45,8 +45,9 @@ $srv->setDispatchMap(array( // let the xml-rpc server know that the method-handler functions expect plain php values $srv->functions_parameters_type = 'phpvals'; -// let code exceptions float all the way to the remote caller as xml-rpc faults - it helps debugging -$srv->exception_handling = 1; +// let code exceptions float all the way to the remote caller as xml-rpc faults - it helps debugging. +// At the same time, it opens a wide security hole, and should never be enabled on public or production servers... +//$srv->exception_handling = 1; // NB: take care not to output anything else after this call, as it will mess up the responses and it will be hard to // debug. In case you have to do so, at least re-emit a correct Content-Length http header (requires output buffering) diff --git a/demo/server/server.php b/demo/server/server.php index 7ede3d2b..4ca667be 100644 --- a/demo/server/server.php +++ b/demo/server/server.php @@ -56,7 +56,7 @@ $s = new Server($signatures, false); $s->setDebug(3); // Out-of-band information: let the client manipulate the server operations. -// We do this to help the testsuite script: do not reproduce in production! +// We do this to help the testsuite script: *** do not reproduce in production or public environments! *** if (defined('TESTMODE')) { if (isset($_GET['RESPONSE_ENCODING'])) { $s->response_charset_encoding = $_GET['RESPONSE_ENCODING']; diff --git a/doc/manual/phpxmlrpc_manual.adoc b/doc/manual/phpxmlrpc_manual.adoc index 2745f47e..9976c08f 100644 --- a/doc/manual/phpxmlrpc_manual.adoc +++ b/doc/manual/phpxmlrpc_manual.adoc @@ -670,7 +670,9 @@ set in `php.ini`, namely `display_errors`. Another way to prevent echoing of err facilitate debugging is to use the server's `SetDebug` method with debug level 3 (see <>). Exceptions thrown during execution of handler functions are caught by default and an XML-RPC error response is generated -instead. This behaviour can be fine-tuned by usage of the `$exception_handling` server property (see <>). +instead. This behaviour can be fine-tuned by usage of the `$exception_handling` server property (see <>); +please be aware that allowing publicly accessible servers to return the information from php exceptions as part of +the xml-rpc response is a sure way to get hacked. ===== Manual type conversion @@ -761,7 +763,6 @@ $srv = new Server( false ); $srv->functions_parameters_type = 'phpvals'; -$srv->exception_handling = 1; $srv->service(); ---- @@ -769,7 +770,8 @@ There are a few things to keep in mind when using this calling convention: * to return an xml-rpc error, the method handler function must return an instance of Response. The only other way for the server to know when an error response should be served to the client is to throw an exception and set the server's - `exception_handling` member var to 1 (as shown above); + `exception_handling` member var to 1 (but please not that this is generally a _very bad idea_ for servers with public + access); * to return a base64 value, the method handler function must encode it on its own, creating an instance of a Value object; @@ -864,7 +866,8 @@ Note that the ZLIB php extension must be installed for this to work. If it is, ` This property controls the behaviour of the server when an exception is thrown by a method handler php function. Valid values: 0,1,2, with 0 being the default. At level 0, the server catches the exception and returns an 'internal error' xml-rpc response; at 1 it catches the exception and returns an xml-rpc response with the error code and error message -corresponding to the exception that was thrown; at 2, the exception is floated to the upper layers in the code. +corresponding to the exception that was thrown - never enable it for publicly accessible servers!; at 2, the exception +is floated to the upper layers in the code - which hopefully do not display it to end users. ===== $response_charset_encoding @@ -1366,7 +1369,7 @@ $srv = new Server($methods); Please note that similar results to the above, i.e. adding to the server's dispatch map an existing php function which is not aware of xml-rpc, can be obtained without the Wrapper class and the need for introspection, simply by setting -`+$server->unctions_parameters_type = 'phpvals'+` and `+$server->exception_handling = 1+` (see chapter <>). +`+$server->unctions_parameters_type = 'phpvals'+` (see chapter <>). The main difference is that, using the Wrapper class, you get for free the documentation for the xmlrpc-method. ==== wrapPhpClass diff --git a/src/Server.php b/src/Server.php index 3efa1abc..3c3cd296 100644 --- a/src/Server.php +++ b/src/Server.php @@ -37,6 +37,7 @@ class Server /** * @var int * Controls whether the server is going to echo debugging messages back to the client as comments in response body. + * SECURITY SENSITIVE! * Valid values: * 0 = * 1 = @@ -49,7 +50,8 @@ class Server * @var int * Controls behaviour of server when the invoked method-handler function throws an exception (within the `execute` method): * 0 = catch it and return an 'internal error' xmlrpc response (default) - * 1 = catch it and return an xmlrpc response with the error corresponding to the exception + * 1 = SECURITY SENSITIVE DO NOT ENABLE ON PUBLIC SERVERS!!! catch it and return an xmlrpc response with the error + * corresponding to the exception, both its code and message. * 2 = allow the exception to float to the upper layers */ public $exception_handling = 0; @@ -799,7 +801,7 @@ class Server } } } catch (\Exception $e) { - // (barring errors in the lib) an uncatched exception happened in the called function, we wrap it in a + // (barring errors in the lib) an uncaught exception happened in the called function, we wrap it in a // proper error-response switch ($this->exception_handling) { case 2: @@ -822,7 +824,7 @@ class Server $r = new Response(0, PhpXmlRpc::$xmlrpcerr['server_error'], PhpXmlRpc::$xmlrpcstr['server_error']); } } catch (\Error $e) { - // (barring errors in the lib) an uncatched exception happened in the called function, we wrap it in a + // (barring errors in the lib) an uncaught exception happened in the called function, we wrap it in a // proper error-response switch ($this->exception_handling) { case 2: -- 2.47.0