Everything

php

Home » Project » Newsgroups » php.internals » Error behaviour for max_input_vars »

Error behaviour for max_input_vars

php.internals

Mel Dafert

3 years ago

Hi internals,

I recently ran into issues with the ini setting `max_input_vars`.
By default, it will truncate input variables in `$_POST` etc. to the
first 1000, and issue a E_WARNING.

The form I was using had an extremely big multiselect, where in one case
around 1000 options were actively selected,
each selected option being sent to the server as a separate
`my_select[]` POST variable.
The select was the final field in the form, thus is was not immediately
noticed when `max_input_vars` truncated options from the select.

I have since changed `max_input_vars` to a higher value, but there was
nevertheless an unknown amount of data loss in the meantime.

I would have strongly preferred if the request had been aborted with an
error, instead of issuing a warning and continuing with incomplete
data.
However, I could not find a way to reliably detect a way that input
variables had been truncated. There are some suggested solutions in [1],
but all of them have downsides:
- Counting variables inside `$_POST`:
    If there are multiple variables with the same name (without `[]` at
the end), they will be counted for `max_input_vars`, but only show up
once in `$_POST`.
    (This happens in the legacy application I am working on, I do not
see it changing anytime soon.)
- Checking `php://input`:
    This is not possible if the request uses `multipart/form-data`.
- Checking `error_get_last()` for the warning:
    This seems extremely brittle - even if currently there is no other
warning thrown at startup, if that changes in the future, this might break.
- Custom error handler using `set_error_handler()`:
    This is not possible, as the warning is thrown before any PHP code
can run (and register the handler).

If I missed a reliable method, I would be thankful, and this email can
be ignored.

In summary, I believe this can only be solved inside of PHP itself, by
allowing to configure a way for `max_input_vars` to abort the request
instead of truncating the input.
The options I see feasible are:
- A new ini setting `max_input_vars_abort` (default to 0), which, if set
to 1, will abort the request if there are more input variables than allowed.
- A method to reliably detect whether the input vars were truncated (eg.
`function has_post_been_truncated(): bool`), so the application can
decide whether to abort or not.
- Deciding that `max_input_vars` is not relevant anymore and should be
handled by the likes of Apache and NGINX, thus changing the default to
`0` and removing the setting
    over a deprecation period.

I am leaning towards the first option, but would be open to either outcome.

Regards,

Mel Dafert

[1]:
https://stackoverflow.com/questions/12169818/in-php-how-can-i-detect-that-input-vars-were-truncated-due-to-max-input-vars-be

Deleu

3 years ago

On Tue, Sep 13, 2022, 7:58 PM Mel Dafert <mel@dafert.at> wrote:

> The options I see feasible are:
> - A new ini setting `max_input_vars_abort` (default to 0), which, if set
> to 1, will abort the request if there are more input variables than
> allowed.
> - A method to reliably detect whether the input vars were truncated (eg.
> `function has_post_been_truncated(): bool`), so the application can
> decide whether to abort or not.
> - Deciding that `max_input_vars` is not relevant anymore and should be
> handled by the likes of Apache and NGINX, thus changing the default to
> `0` and removing the setting
>      over a deprecation period.

Creating more ini settings is not very exciting. I do really like option 2
though.

Jordan LeDoux

3 years ago

On Tue, Sep 13, 2022 at 10:59 AM Mel Dafert <mel@dafert.at> wrote:

>
>      (This happens in the legacy application I am working on, I do not
> see it changing anytime soon.)
>
>

All of these solutions would be available in some future version of PHP.
While your situation highlights something that might be broadly applicable
to PHP, you'd likely have to update the legacy application to take
advantage of any solution that was eventually included so that it could run
on the most recent version of PHP. Unless you're suggesting that this would
also be backported? 7.4 falls out of all support in a few months.

Just want to make sure you're aware of how the inclusion process would work
for any solution to this issue.

Jordan

Mel Dafert

3 years ago

On 13.09.22 20:29, Jordan LeDoux wrote:

>
>
> On Tue, Sep 13, 2022 at 10:59 AM Mel Dafert <mel@dafert.at> wrote:
>
>
>          (This happens in the legacy application I am working on, I do
>     not
>     see it changing anytime soon.)
>
>
> All of these solutions would be available in some future version of
> PHP. While your situation highlights something that might be broadly
> applicable to PHP, you'd likely have to update the legacy application
> to take advantage of any solution that was eventually included so that
> it could run on the most recent version of PHP. Unless you're
> suggesting that this would also be backported? 7.4 falls out of all
> support in a few months.
>
> Just want to make sure you're aware of how the inclusion process would
> work for any solution to this issue.
>
> Jordan

Hello,

I am aware that this will not help me immediately - I already put some
hack in there that combines multiple of the given detection options and
hopes for the best.
I am not arguing for this to be backported, the third option would
probably only change any default in PHP9 (but give me the knowledge that
I can safely set `max_input_vars` to 0 everywhere).

However, the application is on its way towards being ported to PHP8, and
mostly runs already. But just because I ported all of the backend code
doesn't mean I was able to clean up any frontend code (or overly complex
forms) at the same time.
If any of my proposed options make it into 8.3, I do think it is
realistic that I could use that solution in a two years or so (and sleep
safely knowing that none of my forms containing multiselects will
(almost) silently truncate data).

Regards,

Mel

juan carlos morales

3 years ago

El mar., 13 de septiembre de 2022 14:58, Mel Dafert <mel@dafert.at>
escribió:

>
> In summary, I believe this can only be solved inside of PHP itself, by
> allowing to configure a way for `max_input_vars` to abort the request
> instead of truncating the input.
> The options I see feasible are:
> - A new ini setting `max_input_vars_abort` (default to 0), which, if set
> to 1, will abort the request if there are more input variables than
> allowed.
> - A method to reliably detect whether the input vars were truncated (eg.
> `function has_post_been_truncated(): bool`), so the application can
> decide whether to abort or not.
> - Deciding that `max_input_vars` is not relevant anymore and should be
> handled by the likes of Apache and NGINX, thus changing the default to
> `0` and removing the setting
>      over a deprecation period.
>
> I am leaning towards the first option, but would be open to either outcome.
>

We should not delete the ini setting "max_input_vars"... Is a breaking
change very hard.

I Am in favour of adding More flexibility about how to handle this
situation... And I also think that options 1 and 2 can coexist smoothly.

I suggest you write and RFC for this and continue the discussion on this
e-mail list but with the RFC already created.

juan carlos morales

3 years ago

El mar., 13 de septiembre de 2022 15:33, juan carlos morales <
dev.juan.morales@gmail.com> escribió:

>
>
> El mar., 13 de septiembre de 2022 14:58, Mel Dafert <mel@dafert.at>
> escribió:
>
>>
>> In summary, I believe this can only be solved inside of PHP itself, by
>> allowing to configure a way for `max_input_vars` to abort the request
>> instead of truncating the input.
>> The options I see feasible are:
>> - A new ini setting `max_input_vars_abort` (default to 0), which, if set
>> to 1, will abort the request if there are more input variables than
>> allowed.
>> - A method to reliably detect whether the input vars were truncated (eg.
>> `function has_post_been_truncated(): bool`), so the application can
>> decide whether to abort or not.
>> - Deciding that `max_input_vars` is not relevant anymore and should be
>> handled by the likes of Apache and NGINX, thus changing the default to
>> `0` and removing the setting
>>      over a deprecation period.
>>
>> I am leaning towards the first option, but would be open to either
>> outcome.
>>
>
>
> We should not delete the ini setting "max_input_vars"... Is a breaking
> change very hard.
>
> I Am in favour of adding More flexibility about how to handle this
> situation... And I also think that options 1 and 2 can coexist smoothly.
>
> I suggest you write and RFC for this and continue the discussion on this
> e-mail list but with the RFC already created.
>

Check this out

https://wiki.php.net/rfc/howto

Derick Rethans

3 years ago

On 13 September 2022 19:36:15 BST, juan carlos morales <dev.juan.morales@gmail.com> wrote:

>El mar., 13 de septiembre de 2022 15:33, juan carlos morales <
>dev.juan.morales@gmail.com> escribió:
>
>>
>>
>> El mar., 13 de septiembre de 2022 14:58, Mel Dafert <mel@dafert.at>
>> escribió:
>>
>>>
>>> In summary, I believe this can only be solved inside of PHP itself, by
>>> allowing to configure a way for `max_input_vars` to abort the request
>>> instead of truncating the input.
>>> The options I see feasible are:
>>> - A new ini setting `max_input_vars_abort` (default to 0), which, if set
>>> to 1, will abort the request if there are more input variables than
>>> allowed.
>>> - A method to reliably detect whether the input vars were truncated (eg.
>>> `function has_post_been_truncated(): bool`), so the application can
>>> decide whether to abort or not.
>>> - Deciding that `max_input_vars` is not relevant anymore and should be
>>> handled by the likes of Apache and NGINX, thus changing the default to
>>> `0` and removing the setting
>>>      over a deprecation period.
>>>
>>> I am leaning towards the first option, but would be open to either
>>> outcome.
>>>
>>
>>
>> We should not delete the ini setting "max_input_vars"... Is a breaking
>> change very hard.
>>
>> I Am in favour of adding More flexibility about how to handle this
>> situation... And I also think that options 1 and 2 can coexist smoothly.
>>
>> I suggest you write and RFC for this and continue the discussion on this
>> e-mail list but with the RFC already created.
>>
>
>
>Check this out
>
>https://wiki.php.net/rfc/howto

That's quite a condescending thing to say, considering that Mel has already successfully passed an RFC (https://wiki.php.net/rfc/intldatetimepatterngenerator).

cheers
Derick

juan carlos morales

3 years ago

El mar., 13 de septiembre de 2022 20:01, Derick Rethans <derick@php.net>
escribió:

> On 13 September 2022 19:36:15 BST, juan carlos morales <
> dev.juan.morales@gmail.com> wrote:
> >El mar., 13 de septiembre de 2022 15:33, juan carlos morales <
> >dev.juan.morales@gmail.com> escribió:
> >
> >>
> >>
> >> El mar., 13 de septiembre de 2022 14:58, Mel Dafert <mel@dafert.at>
> >> escribió:
> >>
> >>>
> >>> In summary, I believe this can only be solved inside of PHP itself, by
> >>> allowing to configure a way for `max_input_vars` to abort the request
> >>> instead of truncating the input.
> >>> The options I see feasible are:
> >>> - A new ini setting `max_input_vars_abort` (default to 0), which, if
> set
> >>> to 1, will abort the request if there are more input variables than
> >>> allowed.
> >>> - A method to reliably detect whether the input vars were truncated
> (eg.
> >>> `function has_post_been_truncated(): bool`), so the application can
> >>> decide whether to abort or not.
> >>> - Deciding that `max_input_vars` is not relevant anymore and should be
> >>> handled by the likes of Apache and NGINX, thus changing the default to
> >>> `0` and removing the setting
> >>>      over a deprecation period.
> >>>
> >>> I am leaning towards the first option, but would be open to either
> >>> outcome.
> >>>
> >>
> >>
> >> We should not delete the ini setting "max_input_vars"... Is a breaking
> >> change very hard.
> >>
> >> I Am in favour of adding More flexibility about how to handle this
> >> situation... And I also think that options 1 and 2 can coexist smoothly.
> >>
> >> I suggest you write and RFC for this and continue the discussion on this
> >> e-mail list but with the RFC already created.
> >>
> >
> >
> >Check this out
> >
> >https://wiki.php.net/rfc/howto
>
> That's quite a condescending thing to say, considering that Mel has
> already successfully passed an RFC (
> https://wiki.php.net/rfc/intldatetimepatterngenerator).
>
> cheers
> Derick
>

I did not know that :=)
You know My intention was to be supportive 100% :=)

Thanks for pointing It. Have a good day

Jordan LeDoux

3 years ago

On Tue, Sep 13, 2022 at 4:01 PM Derick Rethans <derick@php.net> wrote:

> On 13 September 2022 19:36:15 BST, juan carlos morales <
> dev.juan.morales@gmail.com> wrote:
> >El mar., 13 de septiembre de 2022 15:33, juan carlos morales <
> >dev.juan.morales@gmail.com> escribió:
> >
> >>
> >>
> >> El mar., 13 de septiembre de 2022 14:58, Mel Dafert <mel@dafert.at>
> >> escribió:
> >>
> >>>
> >>> In summary, I believe this can only be solved inside of PHP itself, by
> >>> allowing to configure a way for `max_input_vars` to abort the request
> >>> instead of truncating the input.
> >>> The options I see feasible are:
> >>> - A new ini setting `max_input_vars_abort` (default to 0), which, if
> set
> >>> to 1, will abort the request if there are more input variables than
> >>> allowed.
> >>> - A method to reliably detect whether the input vars were truncated
> (eg.
> >>> `function has_post_been_truncated(): bool`), so the application can
> >>> decide whether to abort or not.
> >>> - Deciding that `max_input_vars` is not relevant anymore and should be
> >>> handled by the likes of Apache and NGINX, thus changing the default to
> >>> `0` and removing the setting
> >>>      over a deprecation period.
> >>>
> >>> I am leaning towards the first option, but would be open to either
> >>> outcome.
> >>>
> >>
> >>
> >> We should not delete the ini setting "max_input_vars"... Is a breaking
> >> change very hard.
> >>
> >> I Am in favour of adding More flexibility about how to handle this
> >> situation... And I also think that options 1 and 2 can coexist smoothly.
> >>
> >> I suggest you write and RFC for this and continue the discussion on this
> >> e-mail list but with the RFC already created.
> >>
> >
> >
> >Check this out
> >
> >https://wiki.php.net/rfc/howto
>
> That's quite a condescending thing to say, considering that Mel has
> already successfully passed an RFC (
> https://wiki.php.net/rfc/intldatetimepatterngenerator).
>
> cheers
> Derick
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

I didn't know that either! That also makes my comment about version
inclusion a bit condescending. Sorry Mel!

Jordan

Côme Chilliet

3 years ago

Hello,

Le mardi 13 septembre 2022, 19:58:42 CEST Mel Dafert a écrit :

> Hi internals,
>
> I recently ran into issues with the ini setting `max_input_vars`.
> By default, it will truncate input variables in `$_POST` etc. to the
> first 1000, and issue a E_WARNING.

I also ran into this a few years ago and it is really annoying. I agree we
need a reliable way of catching this error.

> In summary, I believe this can only be solved inside of PHP itself, by
> allowing to configure a way for `max_input_vars` to abort the request
> instead of truncating the input.
> The options I see feasible are:
> - A new ini setting `max_input_vars_abort` (default to 0), which, if set
> to 1, will abort the request if there are more input variables than allowed.
> - A method to reliably detect whether the input vars were truncated (eg.
> `function has_post_been_truncated(): bool`), so the application can decide
> whether to abort or not.
> - Deciding that `max_input_vars` is not relevant anymore and should be
> handled by the likes of Apache and NGINX, thus changing the default to
> `0` and removing the setting
>      over a deprecation period.

All 3 solutions seems a nice improvement from current situation.

Côme

Thomas Nunninger

3 years ago

Hi,

> In summary, I believe this can only be solved inside of PHP itself, by
> allowing to configure a way for `max_input_vars` to abort the request
> instead of truncating the input.
> The options I see feasible are:
> - A new ini setting `max_input_vars_abort` (default to 0), which, if set
> to 1, will abort the request if there are more input variables than
> allowed.
> - A method to reliably detect whether the input vars were truncated (eg.
> `function has_post_been_truncated(): bool`), so the application can
> decide whether to abort or not.
> - Deciding that `max_input_vars` is not relevant anymore and should be
> handled by the likes of Apache and NGINX, thus changing the default to
> `0` and removing the setting
>      over a deprecation period.
>
> I am leaning towards the first option, but would be open to either outcome.

I'd prefer that PHP aborts such requests. Then data loss/inconsistency
is prevented for everybody and people can fix their applications. (So no
need for an ini setting that allows acting in "danger mode".)

If you'd like to give developers more options to choose from, I'd go for
max_input_vars_abort (default 1) plus has_post_been_truncated(): That
way the behavior is safe from the start. And people who opt in for
"danger mode" can reliably detect if there was some data loss and can
deal with it.

Regards
Thomas

Mel Dafert

3 years ago

On 14 September 2022 16:44:33 CEST, Thomas Nunninger <thomas@nunninger.info> wrote:

>Hi,
>
>> In summary, I believe this can only be solved inside of PHP itself, by allowing to configure a way for `max_input_vars` to abort the request instead of truncating the input.
>> The options I see feasible are:
>> - A new ini setting `max_input_vars_abort` (default to 0), which, if set to 1, will abort the request if there are more input variables than allowed.
>> - A method to reliably detect whether the input vars were truncated (eg. `function has_post_been_truncated(): bool`), so the application can decide whether to abort or not.
>> - Deciding that `max_input_vars` is not relevant anymore and should be handled by the likes of Apache and NGINX, thus changing the default to `0` and removing the setting
>>      over a deprecation period.
>>
>> I am leaning towards the first option, but would be open to either outcome.
>
>I'd prefer that PHP aborts such requests. Then data loss/inconsistency is prevented for everybody and people can fix their applications. (So no need for an ini setting that allows acting in "danger mode".)
>
>If you'd like to give developers more options to choose from, I'd go for max_input_vars_abort (default 1) plus has_post_been_truncated(): That way the behavior is safe from the start. And people who opt in for "danger mode" can reliably detect if there was some data loss and can deal with it.
>
>Regards
>Thomas
>

That's a fourth option that I had overlooked: Just changing the behaviour to always abort, without the option to truncate.
This would certainly be acceptable to me.
Is there anyone relying on the truncating behavior? It's hard for me to imagine such a situation.
This question also determines whether this would be acceptable to go into 8.3, or if we would need to wait for 9. Is something like this considered a breaking change?

This reasoning would also affect your second proposal - changing the default is similarly a breaking change if there are people relying on it, albeit a little easier to fix.

If people think it would be okay, then I would strongly prefer option 4, as then there's no need for bikeshedding on ini settings or new global functions.

Regards,
Mel

Larry Garfield

3 years ago

On Wed, Sep 14, 2022, at 12:07 PM, Mel Dafert wrote:

> On 14 September 2022 16:44:33 CEST, Thomas Nunninger
> <thomas@nunninger.info> wrote:
>>Hi,
>>
>>> In summary, I believe this can only be solved inside of PHP itself, by allowing to configure a way for `max_input_vars` to abort the request instead of truncating the input.
>>> The options I see feasible are:
>>> - A new ini setting `max_input_vars_abort` (default to 0), which, if set to 1, will abort the request if there are more input variables than allowed.
>>> - A method to reliably detect whether the input vars were truncated (eg. `function has_post_been_truncated(): bool`), so the application can decide whether to abort or not.
>>> - Deciding that `max_input_vars` is not relevant anymore and should be handled by the likes of Apache and NGINX, thus changing the default to `0` and removing the setting
>>>      over a deprecation period.
>>>
>>> I am leaning towards the first option, but would be open to either outcome.
>>
>>I'd prefer that PHP aborts such requests. Then data loss/inconsistency is prevented for everybody and people can fix their applications. (So no need for an ini setting that allows acting in "danger mode".)
>>
>>If you'd like to give developers more options to choose from, I'd go for max_input_vars_abort (default 1) plus has_post_been_truncated(): That way the behavior is safe from the start. And people who opt in for "danger mode" can reliably detect if there was some data loss and can deal with it.
>>
>>Regards
>>Thomas
>>
>
> That's a fourth option that I had overlooked: Just changing the
> behaviour to always abort, without the option to truncate.
> This would certainly be acceptable to me.
> Is there anyone relying on the truncating behavior? It's hard for me to
> imagine such a situation.
> This question also determines whether this would be acceptable to go
> into 8.3, or if we would need to wait for 9. Is something like this
> considered a breaking change?
>
> This reasoning would also affect your second proposal - changing the
> default is similarly a breaking change if there are people relying on
> it, albeit a little easier to fix.
>
> If people think it would be okay, then I would strongly prefer option
> 4, as then there's no need for bikeshedding on ini settings or new
> global functions.
>
> Regards,
> Mel

I think the key question here is if there is a reasonable action the developer could take if an over-sized request came in.  PHP itself can dump that to the log, but is there anything reasonable beyond that the developer could do, if they could detect it?

And is anyone doing that now?

--Larry Garfield

Jordan LeDoux

3 years ago

On Wed, Sep 14, 2022 at 11:38 AM Larry Garfield <larry@garfieldtech.com>
wrote:

>
> I think the key question here is if there is a reasonable action the
> developer could take if an over-sized request came in.  PHP itself can dump
> that to the log, but is there anything reasonable beyond that the developer
> could do, if they could detect it?
>
> And is anyone doing that now?
>
> --Larry Garfield
>
>

Honestly, another question I'm thinking about at the moment is whether it's
possible to construct an attack against known script behavior if you also
are able to determine the ini config at which partial form data would make
it to the script with the script thinking it has full form data. To be
clear, I haven't been able to think of one, but I also recognize that I'm
not nearly as clever at those sorts of things as some attackers are.

I suppose that would depend on both the form and the script though.

Jordan

Tim Düsterhus

3 years ago

Hi

On 9/14/22 20:44, Jordan LeDoux wrote:

> Honestly, another question I'm thinking about at the moment is whether it's
> possible to construct an attack against known script behavior if you also
> are able to determine the ini config at which partial form data would make
> it to the script with the script thinking it has full form data. To be
> clear, I haven't been able to think of one, but I also recognize that I'm
> not nearly as clever at those sorts of things as some attackers are.

Maybe I misunderstood what you are thinking about, but can't you just …
not send all the fields to achieve exactly the same results as an attacker?

Best regards
Tim Düsterhus

Jordan LeDoux

3 years ago

On Wed, Sep 14, 2022 at 12:33 PM Tim Düsterhus <tim@bastelstu.be> wrote:

> Hi
>
> On 9/14/22 20:44, Jordan LeDoux wrote:
> > Honestly, another question I'm thinking about at the moment is whether
> it's
> > possible to construct an attack against known script behavior if you also
> > are able to determine the ini config at which partial form data would
> make
> > it to the script with the script thinking it has full form data. To be
> > clear, I haven't been able to think of one, but I also recognize that I'm
> > not nearly as clever at those sorts of things as some attackers are.
>
> Maybe I misunderstood what you are thinking about, but can't you just …
> not send all the fields to achieve exactly the same results as an attacker?
>
> Best regards
> Tim Düsterhus
>

Yes, probably. That's why I was saying, I know I'm not as clever with that
space. I think those would be equivalent cases, but I'm not sure if there
are any edgecases there either. Maybe that thought wasn't appropriate for
the ML, since I'm not suggesting there is a problem, I'm mostly just
wondering if someone with more expertise can confirm that it isn't an issue.

Jordan

Tim Düsterhus

3 years ago

Hi

On 9/14/22 21:38, Jordan LeDoux wrote:

> the ML, since I'm not suggesting there is a problem, I'm mostly just
> wondering if someone with more expertise can confirm that it isn't an issue.
>

As indicated by the phrasing in my previous email, this knowledge does
not enable an attacker to do anything that they wouldn't be able to do
otherwise. I would also expect that at least the value of the INI
setting is going to be the default value of 1000 for the vast majority
of installations out there.

This is primarily an issue of user experience: A user's input data might
not be correctly processed without the user or the PHP application being
aware of it.

This incorrect processing might have security implications, e.g. when an
application uses checkboxes to remove users from a group with elevated
permissions, the admin checks more than 1000 users and the application
does not remove the excess users from the group, despite the user making
extra sure to double check that they checked all the users.

Thus to answer Larry's question, a reasonable action the script could
take is: Show a non-technical well-styled error message to the human,
instead of aborting the request with a 500 or silently causing "data
corruption".

Best regards
Tim Düsterhus

Craig Francis

3 years ago

On 14 Sep 2022, at 20:55, Tim Düsterhus <tim@bastelstu.be> wrote:

> As indicated by the phrasing in my previous email, this knowledge does not enable an attacker to do anything that they wouldn't be able to do otherwise.

One possibility... when you say the attacker is able to "not send all the fields", would that be via injecting malicious JavaScript? which would hopefully be blocked via the websites Content Security Policy?... a different approach could use a simple XSS within the <form>, and injecting ~995 hidden <input> fields:

```
<form action="/user/edit" method="post">
[...]
<input type="hidden" name="xss[]" />
<input type="hidden" name="xss[]" />
<input type="hidden" name="xss[]" />
[...]
<label>
<input type="checkbox" name="group_delete[]" value="1" />
Remove from Group 1
</label>
[...]
</form>
```

Craig

Tim Düsterhus

3 years ago

Hi

On 9/13/22 19:58, Mel Dafert wrote:

> - Deciding that `max_input_vars` is not relevant anymore and should be
> handled by the likes of Apache and NGINX, thus changing the default to
> `0` and removing the setting
>       over a deprecation period.

This would be my preferred option, but my understanding is that the
limit still is relevant to protect against attacks on the hash table
implementation. The web server can't really protect against this type of
attack, because the payload required to execute the attack is fairly
small. Protecting against the attack without some arbitrary cut-off
limit would require making the hash algorithm used for the superglobals
dependent on a randomly generated per-request seed value. I can't
comment on how easy or hard that would be to change, but I believe that
this should be the ultimate goal here. It's also what other programming
languages do.

Best regards
Tim Düsterhus

Mel Dafert

3 years ago

On 14 September 2022 21:39:36 CEST, "Tim Düsterhus" <tim@bastelstu.be> wrote:

>Hi
>
>On 9/13/22 19:58, Mel Dafert wrote:
>> - Deciding that `max_input_vars` is not relevant anymore and should be
>> handled by the likes of Apache and NGINX, thus changing the default to
>> `0` and removing the setting
>>       over a deprecation period.
>
>This would be my preferred option, but my understanding is that the limit still is relevant to protect against attacks on the hash table implementation. The web server can't really protect against this type of attack, because the payload required to execute the attack is fairly small. Protecting against the attack without some arbitrary cut-off limit would require making the hash algorithm used for the superglobals dependent on a randomly generated per-request seed value. I can't comment on how easy or hard that would be to change, but I believe that this should be the ultimate goal here. It's also what other programming languages do.
>
>Best regards
>Tim Düsterhus

Thank you, this makes a lot of sense to me.
I assume that this rules out that option, at least for now, unless someone makes the relevant changes to the hashing.
But this also means that aborting the request would be just as effective at protecting such an attack as truncating.

Would aborting instead of truncating introduce any new attack surface?
The only thing I could think of would be a DoS vector, but I believe there are a lot of ways you can get a server to instantly abort your request...

Regards,
Mel

Tim Düsterhus

3 years ago

Hi

On 9/14/22 21:58, Mel Dafert wrote:

> Thank you, this makes a lot of sense to me.
> I assume that this rules out that option, at least for now, unless someone makes the relevant changes to the hashing.

You are assuming correctly. That limit was specifically introduced to
protect against this attack and is specifically documented as such in
https://www.php.net/manual/en/info.configuration.php#ini.max-input-vars:

> Use of this directive mitigates the possibility of denial of service

attacks which use hash collisions.

It *might* be feasible to raise the default value nowadays, because of
increased processing power moving the point where it "gets bad" further
upwards.

-----

> But this also means that aborting the request would be just as effective at protecting such an attack as truncating.

Yes. It might or might not be worse with regard to user experience,
though (not that the truncating behavior is any good either). See also
my reply to Jordan's email.

> Would aborting instead of truncating introduce any new attack surface?

I don't see how that would add to the attack surface if implemented
correctly.

> The only thing I could think of would be a DoS vector, but I believe there are a lot of ways you can get a server to instantly abort your request...

If you are only able to abort processing of your own request, then it's
not even a DoS vector, as you don't affect other users and processing a
regular request certainly is going to be more expensive on the server
(database connections and similar) compared to aborting before your
script starts executing.

Best regards
Tim Düsterhus

Claude Pache

3 years ago

> Le 13 sept. 2022 à 19:58, Mel Dafert <mel@dafert.at> a écrit :
>
> In summary, I believe this can only be solved inside of PHP itself, by allowing to configure a way for `max_input_vars` to abort the request instead of truncating the input.
> The options I see feasible are:
> - A new ini setting `max_input_vars_abort` (default to 0), which, if set to 1, will abort the request if there are more input variables than allowed.
> - A method to reliably detect whether the input vars were truncated (eg. `function has_post_been_truncated(): bool`), so the application can decide whether to abort or not.
> - Deciding that `max_input_vars` is not relevant anymore and should be handled by the likes of Apache and NGINX, thus changing the default to `0` and removing the setting
>     over a deprecation period.
>
> I am leaning towards the first option, but would be open to either outcome.

Hi,

A big issue with `max_input_vars` (and indeed any fallible procedure executed at startup) is that errors occur before that the program has a chance to install a custom error/exception handler or to open a try/catch block.

For the specific case of $_POST, a possible option would be:

* set the ini setting `enable_post_data_reading` to false, so that $_POST and $_FILES are not populated at startup;
* have a function that to enable to populate those variables at runtime:

```php
try {
read_post_data([ 'max_input_vars' => 100_000 ]);
}
catch (\TooManyInputVarsException) {
// custom panic procedure
}
```

A more general solution would be to be able to retrieve all errors (not just the last) that has been triggered at startup, e.g.:

```php
set_error_handler('app_specific_error_handler');
replay_error_triggered_at_startup(); // will re-trigger all errors that has occurred before the first line of code
```

—Claude