Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A way to disable some automatic body parsing #106

Open
cypherbits opened this issue Dec 14, 2019 · 7 comments
Open

A way to disable some automatic body parsing #106

cypherbits opened this issue Dec 14, 2019 · 7 comments

Comments

@cypherbits
Copy link

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.

@cypherbits cypherbits changed the title A way to disable automatic body parsing A way to disable some automatic body parsing Dec 14, 2019
@l0gicgate l0gicgate transferred this issue from slimphp/Slim Dec 17, 2019
@t0mmy742
Copy link
Contributor

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
Copy link

pdscopes commented Feb 26, 2020

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
Copy link
Author

cypherbits commented Feb 26, 2020

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
Copy link
Member

akrabat commented Feb 26, 2020

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
Copy link
Member

akrabat commented Feb 26, 2020

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
Copy link

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.

cypherbits pushed a commit to cypherbits/nullupload that referenced this issue Sep 26, 2020
nullupload-app.ERROR: Call to undefined function Slim\Http\simplexml_load_string() on /var/www/nullupload/vendor/slim/slim/Slim/Http/Request.php at 230
slimphp/Slim-Http#106
@lauri-elevant
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants