slimphp / Slim-Http

A set of PSR-7 object decorators providing useful convenience methods
MIT License
151 stars 47 forks source link

A way to disable some automatic body parsing #106

Open cypherbits opened 4 years ago

cypherbits commented 4 years ago

Hello, Is it possible to disable automatic request parsing?

At least in version 3 it parses like this:

JSON requests are converted into associative arrays with json_decode($input, true). XML requests are converted into a SimpleXMLElement with simplexml_load_string($input). URL-encoded requests are converted into a PHP array with parse_str($input).

But I don't have installed the xml extension or have the functions disabled for security reasons. When an attacker sends a xml requests it wants to parse and I get an error like:

Call to undefined function Slim\Http\simplexml_load_string() on /var/www/website/vendor/slim/slim/Slim/Http/Request.php at 230

I would like to choose what parsing is done from these 3 options.

t0mmy742 commented 4 years ago

You can try this code :

$request->registerMediaTypeParser('application/xml', function ($input) {
    return null;
});
$request->registerMediaTypeParser('text/xml', function ($input) {
    return null;
});

We could also create a function unregisterMediaTypeParser(string $mediaType). We would only have to call this function twice. Or call a function only once, named unregisterMultipleMediaTypeParser(array $mediaType) (I'm not attached to these names).

pdscopes commented 4 years ago

Another simple way to solve this is to add a middleware layer that checks a whitelist of supported content types, e.g.:

// Limit requests to application/json
$app->add(function (Request $request, Response $response, $next) {
    $whitelistContentTypes = ['application/json'];
    if (!in_array($request->getMediaType(), $whitelistContentTypes)) {
        return $response->withJson(['message' => 'Unsupported Media Type'], 415);
    }

    return $next($request, $response);
});

[edit] You may also need to consider the case where $request->getMediaType() === null

cypherbits commented 4 years ago

Hi @pdscopes thanks for the reply, but if I want to just allow plain/normal HTTP messages, how would the code look like? (I mean blacklist XML and/or JSON and/or any other. What is the MediaType set for normal requests? Or maybe a whitelist: just allow URLencoded type.)

akrabat commented 4 years ago

To answer the original question, replace the XML media parser callback with your own.

Something like this will probably work:

$request->registerMediaTypeParser('application/xml', function () { return []; });
$request->registerMediaTypeParser('text/xml', function () { return []; });

(untested!)

akrabat commented 4 years ago

Hi @pdscopes thanks for the reply, but if I want to just allow plain/normal HTTP messages, how would the code look like? (I mean blacklist XML and/or JSON and/or any other. What is the MediaType set for normal requests? Or maybe a whitelist: just allow URLencoded type.)

For normal POSTed form data, the Content-Type should be: application/x-www-form-urlencoded or multipart/form-data.

pdscopes commented 4 years ago

For normal POSTed form data, the Content-Type should be: application/x-www-form-urlencoded or multipart/form-data.

To follow on from this GET requests are unlikely not to send a Content-Type header so the media type will be null.

lauri-elevant commented 2 years ago

The problem is that the request object is immutable, so if you call withWhatever on it, it is re-constructed and the same baked in parsers are re-added to the top-of-the chain request object in the constructor.

So you might disable it, and then someone else does something like withAttribute in a middleware, and you are back to them being enabled. This is a design issue. The parsers should be chainable, so that once set up, they are preserved.