cphalcon icon indicating copy to clipboard operation
cphalcon copied to clipboard

[NFR]: Handling array data in request payload (GET/POST)

Open wurst-hans opened this issue 3 years ago • 1 comments

IMHO there is no solution currently, to handle unexpected array data in $_GET or $_POST.

When validating forms or using query parameters of request, there is no buit-in way (i.e. filter or whatever) to guarantee that we do not process an array when we expect a string.

For example a simple form:

$form = new Form();
$form->add(
    (new Text('firstname'))
);
if ($this->request->isPost() && $form->isValid($_POST)) {
    // ...
}
return $this->view
    ->setVars(['form' => $form])
    ->render('sso/signup');

Will result in

Phalcon\Tag\Exception: Value at index: 'value' type: 'array' cannot be rendered in /var/www/forms%%input.volt.php:14
Stack trace:
#0 [internal function]: Phalcon\Tag::renderAttributes()
#1 [internal function]: Phalcon\Tag::inputField()
#2 [internal function]: Phalcon\Tag::textField()
#3 [internal function]: Phalcon\Forms\Element\Text->render()

When the $_POST payload contains an array firstname[]=hacked instead of a string value firstname=john. This can happen for whatever reason.

Also for simple query data in request this can happen. I've seen some web applications built with Phalcon the last days, which can be crashed with an HTTP 500 error. For example this page has a search slot for reviews. This would add a parameter q to URL. But when manipulating this to q[]= the page will crash, which can be seen here and will throw the following error:

Value at index: 'value' type: 'array' cannot be rendered

That's just one live example. We need any sanitizing filter, middleware or whatever which will filter or flatten any array in request payload. I thought about a middleware attached to application:boot but that's not ideal, because it will remove all arrays from payload, even when we need any array data really. To clarify, imagine a multi checkbox option in a form, like

<input type="checkbox" name="partner[]" value="IBM">
<input type="checkbox" name="partner[]" value="Microsoft">
<input type="checkbox" name="partner[]" value="Adobe">

In this case we must not remove or flatten partner[] from payload, but all other fields need to be filtered, maybe. Additionally, we should be able to define up to which hierarchy level we allow an array value. So, when we would allow partner[] to be accepted as $_POST data, it should be a one-level array only. But when using application:boot middleware there is no point to intercept or set any parameter names which should be excluded from filtering.

Additionally it's very uncomfortable to always sanitize standard query parameters manually, i.e.:

  • q for search term
  • p for pagination
  • s for sorting results

Also, this is true for forms. Currently, I'm adding a custom is_scalar validator to any form field. Really annoying.

It's not really a bug, but as this affects any project that is developed currently, this is highly critical for me. How can this be solved?

wurst-hans avatar May 07 '21 14:05 wurst-hans