FAQ
Hi dmitry:

it seems you have fix the issue that error in register_variable
will cause php process exit.

here is a fix I made before: http://pastebin.com/7BLAVaWr , I
think maybe this is a lighter fix.

could you review this? if you think this is okey, I will commit it.

thanks very much.

--
Laruence  Xinchen Hui
http://www.laruence.com/

Search Discussions

  • Stas Malyshev at Jan 4, 2012 at 7:08 am
    Hi!
    it seems you have fix the issue that error in register_variable
    will cause php process exit.

    here is a fix I made before: http://pastebin.com/7BLAVaWr , I
    think maybe this is a lighter fix.

    could you review this? if you think this is okey, I will commit it.
    Could you please explain what exactly causes process exit and how it can
    be reproduced? I.e., what is the problem in the patch that is committed now?
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Laruence at Jan 4, 2012 at 7:13 am
    Hi:
    sorry, I didn't put that clearly, I mean, in trigger E_ERROR in
    php_register_variable will cause php-cgi process exit.

    and dmitry has fixed that by change the E_ERROR to E_WARNING,

    my patch is doing the same thing but in another way

    thanks
    On Wed, Jan 4, 2012 at 3:08 PM, Stas Malyshev wrote:
    Hi!

    it seems you have fix the issue that error in register_variable
    will cause php process exit.

    here is a fix I made before: http://pastebin.com/7BLAVaWr ,  I
    think maybe this is a lighter fix.

    could you review this?   if you think this is okey,  I will commit it.

    Could you please explain what exactly causes process exit and how it can be
    reproduced? I.e., what is the problem in the patch that is committed now?
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Laruence at Jan 4, 2012 at 10:00 am

    On Wed, Jan 4, 2012 at 2:59 PM, Laruence wrote:
    Hi dmitry:

    it seems you have fix the issue that error in register_variable
    will cause php process exit.

    here is a fix I made before: http://pastebin.com/7BLAVaWr ,  I
    think maybe this is a lighter fix.

    could you review this?   if you think this is okey,  I will commit it.
    Hmm, after a deep thought, this patch will not work in case of sub
    arrays in POST ..

    thanks
    thanks very much.

    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Laruence at Jan 4, 2012 at 11:34 am
    Hi:
    I have updated the patch, make it works in case of sub arrays.

    http://pastebin.com/yPTUZuNe

    thanks
    On Wed, Jan 4, 2012 at 5:59 PM, Laruence wrote:
    On Wed, Jan 4, 2012 at 2:59 PM, Laruence wrote:
    Hi dmitry:

    it seems you have fix the issue that error in register_variable
    will cause php process exit.

    here is a fix I made before: http://pastebin.com/7BLAVaWr ,  I
    think maybe this is a lighter fix.

    could you review this?   if you think this is okey,  I will commit it.
    Hmm, after a deep thought, this patch will not work in case of  sub
    arrays in POST ..

    thanks
    thanks very much.

    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Laruence at Jan 4, 2012 at 11:53 am

    On Wed, Jan 4, 2012 at 7:34 PM, Laruence wrote:
    Hi:
    I have updated the patch, make it works in case of sub arrays.

    http://pastebin.com/yPTUZuNe
    this patch only restrict the post variables number, since GET and
    Cookie all have their length limit.

    and it's also easy to restrict the get or request too(add the samilar
    logic in php_default_treat_data), I just think that is no-needed :)

    thanks
    thanks
    On Wed, Jan 4, 2012 at 5:59 PM, Laruence wrote:
    On Wed, Jan 4, 2012 at 2:59 PM, Laruence wrote:
    Hi dmitry:

    it seems you have fix the issue that error in register_variable
    will cause php process exit.

    here is a fix I made before: http://pastebin.com/7BLAVaWr ,  I
    think maybe this is a lighter fix.

    could you review this?   if you think this is okey,  I will commit it.
    Hmm, after a deep thought, this patch will not work in case of  sub
    arrays in POST ..

    thanks
    thanks very much.

    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Jpauli at Jan 4, 2012 at 1:30 pm

    On Wed, Jan 4, 2012 at 12:52 PM, Laruence wrote:
    On Wed, Jan 4, 2012 at 7:34 PM, Laruence wrote:
    Hi:
    I have updated the patch, make it works in case of sub arrays.

    http://pastebin.com/yPTUZuNe
    this patch only restrict the post variables number, since GET and
    Cookie all have their length limit.

    and it's also easy to restrict the get or request too(add the samilar
    logic in php_default_treat_data), I just think that is no-needed :)

    thanks
    I don't think adding one more .ini option is a good idea.
    That will lead to people confused, and regarding security parameters, that
    is never a good idea.

    For example, people would ask what is the difference between max_input_vars
    and max_post_vars ?

    Julien.Pauli

    thanks
    On Wed, Jan 4, 2012 at 5:59 PM, Laruence wrote:
    On Wed, Jan 4, 2012 at 2:59 PM, Laruence wrote:
    Hi dmitry:

    it seems you have fix the issue that error in register_variable
    will cause php process exit.

    here is a fix I made before: http://pastebin.com/7BLAVaWr , I
    think maybe this is a lighter fix.

    could you review this? if you think this is okey, I will commit
    it.
    Hmm, after a deep thought, this patch will not work in case of sub
    arrays in POST ..

    thanks
    thanks very much.

    --
    Laruence Xinchen Hui
    http://www.laruence.com/


    --
    Laruence Xinchen Hui
    http://www.laruence.com/


    --
    Laruence Xinchen Hui
    http://www.laruence.com/


    --
    Laruence Xinchen Hui
    http://www.laruence.com/

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
  • Pierre Joye at Jan 4, 2012 at 1:59 pm
    hi,

    There is no other option.

    This value is used before a script even get the hand. So we have to
    set a value by default, but we cannot force it, that's why we have to
    use an ini setting.

    Cheers,
    On Wed, Jan 4, 2012 at 2:30 PM, jpauli wrote:
    On Wed, Jan 4, 2012 at 12:52 PM, Laruence wrote:
    On Wed, Jan 4, 2012 at 7:34 PM, Laruence wrote:
    Hi:
    I have updated the patch, make it works in case of sub arrays.

    http://pastebin.com/yPTUZuNe
    this patch only restrict the post variables number, since GET and
    Cookie all have their length limit.

    and it's also easy to restrict the get or request too(add the samilar
    logic in php_default_treat_data),  I just think that is no-needed :)

    thanks
    I don't think adding one more .ini option is a good idea.
    That will lead to people confused, and regarding security parameters, that
    is never a good idea.

    For example, people would ask what is the difference between max_input_vars
    and max_post_vars ?

    Julien.Pauli

    thanks
    On Wed, Jan 4, 2012 at 5:59 PM, Laruence wrote:
    On Wed, Jan 4, 2012 at 2:59 PM, Laruence wrote:
    Hi dmitry:

    it seems you have fix the issue that error in register_variable
    will cause php process exit.

    here is a fix I made before: http://pastebin.com/7BLAVaWr ,  I
    think maybe this is a lighter fix.

    could you review this?   if you think this is okey,  I will commit
    it.
    Hmm, after a deep thought, this patch will not work in case of  sub
    arrays in POST ..

    thanks
    thanks very much.

    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php


    --
    Pierre

    @pierrejoye | http://blog.thepimp.net | http://www.libgd.org
  • Stas Malyshev at Jan 4, 2012 at 5:19 pm
    Hi!
    Hi:
    I have updated the patch, make it works in case of sub arrays.

    http://pastebin.com/yPTUZuNe
    I'm sorry, I'm not sure still I understand - what is this patch fixing?
    I.e. what is the problem with the current PHP that needs this patch?

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Ferenc Kovacs at Jan 4, 2012 at 6:55 pm

    On Wed, Jan 4, 2012 at 6:18 PM, Stas Malyshev wrote:

    Hi!


    Hi:
    I have updated the patch, make it works in case of sub arrays.

    http://pastebin.com/yPTUZuNe
    I'm sorry, I'm not sure still I understand - what is this patch fixing?
    I.e. what is the problem with the current PHP that needs this patch?
    the max_input_vars was introduced to fix
    http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-4885

    My understanding is that the original patch was too intrusive, from the
    comment of Laruence, it seems that throwing an E_ERROR in that phase means
    that you can kill the cgi workers if you pass the malicious input, which
    has still some DOS potential, this seems to be backed as Dmitry changed the
    original patch, to only raise a warning and ignore the vars over the set
    limits: http://svn.php.net/viewvc?view=revision&revision=321335

    From the comments by Laruence it seems that he thinks that we only need to
    limit post, as get and cookie has external limits.
    I disagree with this, for two reasons:
    - we also have post_max_size to limit the length of the Post
    Content-Length, so by the same logic, we wouldn't need an additional ini
    setting for limiting the number of variables.
    - we also have max_input_nesting_level, so having a max_input_vars seems to
    be more consistent, than max_post_vars

    I would also like to point out, that when we added max_input_nesting_level
    we later polished that a little bit:
    http://svn.php.net/viewvc/php/php-src/trunk/main/php_variables.c?r1=233940&r2=236894
    http://svn.php.net/viewvc/php/php-src/trunk/main/php_variables.c?r1=236898&r2=239877

    And I guess at least the information disclosure part would be needed here
    also ( https://twitter.com/#!/i0n1c/status/152356767601393665 )

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Stas Malyshev at Jan 4, 2012 at 6:59 pm
    Hi!
    From the comments by Laruence it seems that he thinks that we only need
    to limit post, as get and cookie has external limits.
    I don't think it's a big problem if we limit all of them. It's not like
    anybody needs a million cookies in their http request.
    And I guess at least the information disclosure part would be needed
    here also ( https://twitter.com/#!/i0n1c/status/152356767601393665 )
    Could you please elaborate on that part - where is the disclosure and
    what exactly is being disclosed?
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Ferenc Kovacs at Jan 4, 2012 at 7:35 pm

    And I guess at least the information disclosure part would be needed
    Could you please elaborate on that part - where is the disclosure and what
    exactly is being disclosed?

    I would guess that the value of that said limit. (it is the only variable
    in the error message).

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Stas Malyshev at Jan 4, 2012 at 7:37 pm
    Hi!
    Could you please elaborate on that part - where is the disclosure
    and what exactly is being disclosed?


    I would guess that the value of that said limit. (it is the only
    variable in the error message).
    This is an error message, it's not visible to anybody. Even if it were,
    I don't see a problem with it. Usually people mean different thing by
    information disclosure, but without proper report of course it is
    meaningless to talk about it.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Ferenc Kovacs at Jan 4, 2012 at 7:46 pm

    On Wed, Jan 4, 2012 at 8:37 PM, Stas Malyshev wrote:

    Hi!


    Could you please elaborate on that part - where is the disclosure
    and what exactly is being disclosed?


    I would guess that the value of that said limit. (it is the only
    variable in the error message).
    This is an error message, it's not visible to anybody. Even if it were, I
    don't see a problem with it. Usually people mean different thing by
    information disclosure, but without proper report of course it is
    meaningless to talk about it.

    /* do not output the error message to the screen,
    this helps us to to avoid "information disclosure" */

    I don't think that it is a high importance, but with display_errors
    enabled, it does leak otherwise unobtainable (if you don't have publicly
    available phpinfo files, which most person with enabled display_errors
    does) info.

    So while I don't feel strongly about it, I wanted to mention it.

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Rasmus Lerdorf at Jan 4, 2012 at 7:50 pm

    On 01/04/2012 11:46 AM, Ferenc Kovacs wrote:
    On Wed, Jan 4, 2012 at 8:37 PM, Stas Malyshev wrote:

    Hi!


    Could you please elaborate on that part - where is the disclosure
    and what exactly is being disclosed?


    I would guess that the value of that said limit. (it is the only
    variable in the error message).
    This is an error message, it's not visible to anybody. Even if it were, I
    don't see a problem with it. Usually people mean different thing by
    information disclosure, but without proper report of course it is
    meaningless to talk about it.

    /* do not output the error message to the screen,
    this helps us to to avoid "information disclosure" */

    I don't think that it is a high importance, but with display_errors
    enabled, it does leak otherwise unobtainable (if you don't have publicly
    available phpinfo files, which most person with enabled display_errors
    does) info.

    So while I don't feel strongly about it, I wanted to mention it.
    Since it is one of these remotely-triggered errors that you can't
    program around, it should probably be suppressed when display_errors is
    on. There is another precedence for this, but I am drawing a blank on
    where else we did this right now.

    -Rasmus
  • Nikita Popov at Jan 4, 2012 at 7:54 pm

    On Wed, Jan 4, 2012 at 8:50 PM, Rasmus Lerdorf wrote:
    Since it is one of these remotely-triggered errors that you can't
    program around, it should probably be suppressed when display_errors is
    on. There is another precedence for this, but I am drawing a blank on
    where else we did this right now.

    -Rasmus
    You mean like with htmlspecialchars() before PHP 5.4? Please don't.
    It's really non-obvious to start hiding errors as soon as you enable
    error display.

    Nikita
  • Rasmus Lerdorf at Jan 4, 2012 at 8:03 pm

    On 01/04/2012 11:54 AM, Nikita Popov wrote:
    On Wed, Jan 4, 2012 at 8:50 PM, Rasmus Lerdorf wrote:
    Since it is one of these remotely-triggered errors that you can't
    program around, it should probably be suppressed when display_errors is
    on. There is another precedence for this, but I am drawing a blank on
    where else we did this right now.

    -Rasmus
    You mean like with htmlspecialchars() before PHP 5.4? Please don't.
    It's really non-obvious to start hiding errors as soon as you enable
    error display.
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written. They
    can check for potential errors and avoid them. This one can't be checked
    for and you could easily write a scanner that scoured the Net for sites
    with display_errors enabled by sending a relatively short POST request
    to each one and checking for this error. And there is absolutely nothing
    people could do about this short of turning off display_errors which we
    know from experience a lot of people just don't do no matter how much we
    suggest it.

    The alternative is to just not have any error message at all. That
    avoids the discrepancy between the error messages with display_errors on
    and off.

    -Rasmus
  • Paul Dragoonis at Jan 4, 2012 at 8:07 pm
    Inline.
    On Wed, Jan 4, 2012 at 8:02 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 11:54 AM, Nikita Popov wrote:
    On Wed, Jan 4, 2012 at 8:50 PM, Rasmus Lerdorf wrote:
    Since it is one of these remotely-triggered errors that you can't
    program around, it should probably be suppressed when display_errors is
    on. There is another precedence for this, but I am drawing a blank on
    where else we did this right now.

    -Rasmus
    You mean like with htmlspecialchars() before PHP 5.4? Please don't.
    It's really non-obvious to start hiding errors as soon as you enable
    error display.
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written. They
    can check for potential errors and avoid them. This one can't be checked
    for and you could easily write a scanner that scoured the Net for sites
    with display_errors enabled by sending a relatively short POST request
    to each one and checking for this error. And there is absolutely nothing
    people could do about this short of turning off display_errors which we
    know from experience a lot of people just don't do no matter how much we
    suggest it.
    I agree with Rasmus here. A lot of people keep display_errors on, even
    when they shouldn't.
    It log_errors is on, it should go to the error_log, but with
    display_errors it should never be sent back to the browser.

    - Paul Dragoonis.
    The alternative is to just not have any error message at all. That
    avoids the discrepancy between the error messages with display_errors on
    and off.

    -Rasmus

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
  • Nikita Popov at Jan 4, 2012 at 8:18 pm

    On Wed, Jan 4, 2012 at 9:07 PM, Paul Dragoonis wrote:
    Inline.
    On Wed, Jan 4, 2012 at 8:02 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 11:54 AM, Nikita Popov wrote:
    On Wed, Jan 4, 2012 at 8:50 PM, Rasmus Lerdorf wrote:
    Since it is one of these remotely-triggered errors that you can't
    program around, it should probably be suppressed when display_errors is
    on. There is another precedence for this, but I am drawing a blank on
    where else we did this right now.

    -Rasmus
    You mean like with htmlspecialchars() before PHP 5.4? Please don't.
    It's really non-obvious to start hiding errors as soon as you enable
    error display.
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written. They
    can check for potential errors and avoid them. This one can't be checked
    for and you could easily write a scanner that scoured the Net for sites
    with display_errors enabled by sending a relatively short POST request
    to each one and checking for this error. And there is absolutely nothing
    people could do about this short of turning off display_errors which we
    know from experience a lot of people just don't do no matter how much we
    suggest it.
    I agree with Rasmus here. A lot of people keep display_errors on, even
    when they shouldn't.
    It log_errors is on, it should go to the error_log, but with
    display_errors it should never be sent back to the browser.
    I simply fear that we are facing the same problem with this as we did
    with htmlspecialchars(). Sure, the behavior was meant nice in the
    first place, but in reality it basically made debugging hard for the
    good guys (you know, with display_errors=Off on production and =On on
    development), because they wouldn't see the error while developing.

    I can imagine the same with this: If you develop some application
    using lots and lots of POST variables (1000+) [for whatever reason]
    and you notice that some of those vars just don't submit you'll have a
    really hard time debugging this. With the warning the issue on the
    other hand would be obvious.

    I really don't like the idea to punish normal developers with hard
    debugging only to make life easier for terrible server admins. I
    understand that as this is a security issue things are a bit more
    complicated, but I still don't really think this is a good idea.

    By the way, an attacker could also find out the limit by measuring
    page load times with various POST data sizes quite easily (I would
    think so at least), so I wouldn't say it's "otherwise unobtainable".

    Nikita
  • Reindl Harald at Jan 4, 2012 at 8:19 pm

    Am 04.01.2012 21:07, schrieb Paul Dragoonis:

    I agree with Rasmus here. A lot of people keep display_errors on, even
    when they shouldn't.
    it is not the job of a programming language stop admins
    from beeing stupid - the defaults have to be sane and this
    is display_error OFF, if somebody decides for whateever
    reason to turn it on it is not yours or anybody others
    decision to ignore the setting here, and there and there also
    but there not
    It log_errors is on, it should go to the error_log, but with
    display_errors it should never be sent back to the browser.
    damned this sort of decisions MUST NOT happen

    if i debug a webiste on a local network and enable
    display_errors you have to display them without any
    fuzzy logic
  • Rasmus Lerdorf at Jan 4, 2012 at 8:56 pm

    On 01/04/2012 12:19 PM, Reindl Harald wrote:


    Am 04.01.2012 21:07, schrieb Paul Dragoonis:
    I agree with Rasmus here. A lot of people keep display_errors
    on, even when they shouldn't.
    it is not the job of a programming language stop admins from
    beeing stupid - the defaults have to be sane and this is
    display_error OFF, if somebody decides for whateever reason to turn
    it on it is not yours or anybody others decision to ignore the
    setting here, and there and there also but there not
    Yes, but display_errors is not off by default, that is the problem. If
    we could get away with turning display_errors off by default, then I
    agree that we don't need this. As it is currently, the default setup,
    if people don't do anything, will result in a security problem because
    of this.

    -Rasmus
  • Ferenc Kovacs at Jan 4, 2012 at 9:01 pm

    On Wed, Jan 4, 2012 at 9:56 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 12:19 PM, Reindl Harald wrote:


    Am 04.01.2012 21:07, schrieb Paul Dragoonis:
    I agree with Rasmus here. A lot of people keep display_errors
    on, even when they shouldn't.
    it is not the job of a programming language stop admins from
    beeing stupid - the defaults have to be sane and this is
    display_error OFF, if somebody decides for whateever reason to turn
    it on it is not yours or anybody others decision to ignore the
    setting here, and there and there also but there not
    Yes, but display_errors is not off by default, that is the problem. If
    we could get away with turning display_errors off by default, then I
    agree that we don't need this. As it is currently, the default setup,
    if people don't do anything, will result in a security problem because
    of this.

    -Rasmus

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
    I just got the tip that this error is only shown if display_startup_errors
    is set to true, and because it is in the startup routine the file path in
    the error message (which is the real infoleak) would only point to "unknown
    0".
    If somebody has the time to double check/test this, it would be nice.

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Rasmus Lerdorf at Jan 4, 2012 at 9:07 pm

    On 01/04/2012 01:01 PM, Ferenc Kovacs wrote:
    I just got the tip that this error is only shown
    if display_startup_errors is set to true, and because it is in the
    startup routine the file path in the error message (which is the real
    infoleak) would only point to "unknown 0".
    If somebody has the time to double check/test this, it would be nice.
    Right, like I said in my previous message, if this is caught by
    display_start_errors, I am ok with it. We need the default/no php.ini
    file case to not leak information like this.

    -Rasmus
  • Stas Malyshev at Jan 4, 2012 at 9:27 pm
    Hi!
    Right, like I said in my previous message, if this is caught by
    display_start_errors, I am ok with it. We need the default/no php.ini
    file case to not leak information like this.
    Just checked - it does not display error if display_startup_errors if
    off, does display if it's on.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Rasmus Lerdorf at Jan 4, 2012 at 9:48 pm

    On 01/04/2012 01:27 PM, Stas Malyshev wrote:
    Hi!
    Right, like I said in my previous message, if this is caught by
    display_start_errors, I am ok with it. We need the default/no php.ini
    file case to not leak information like this.
    Just checked - it does not display error if display_startup_errors if
    off, does display if it's on.
    Right, that seems ok. The other thing is that we need to clarify that it
    actually only limits the number of variables per nesting level. The
    current name and the description doesn't make that clear. You can still
    send 1M post vars in a single POST if you just nest them in a 1000x1000
    2d array. Of course, this is likely going to hit the post_max_size
    limit, although many sites that do file uploads will have cranked that
    way up.

    -Rasmus
  • Rasmus Lerdorf at Jan 4, 2012 at 9:56 pm

    On 01/04/2012 01:48 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 01:27 PM, Stas Malyshev wrote:
    Hi!
    Right, like I said in my previous message, if this is caught by
    display_start_errors, I am ok with it. We need the default/no php.ini
    file case to not leak information like this.
    Just checked - it does not display error if display_startup_errors if
    off, does display if it's on.
    Right, that seems ok. The other thing is that we need to clarify that it
    actually only limits the number of variables per nesting level. The
    current name and the description doesn't make that clear. You can still
    send 1M post vars in a single POST if you just nest them in a 1000x1000
    2d array. Of course, this is likely going to hit the post_max_size
    limit, although many sites that do file uploads will have cranked that
    way up.
    Oh, and a final issue to address.

    This code:

    for($data=[],$i=0; $i<=999; $i++) $data[$i] = range(0,1001);
    echo curl_post("http://localhost/index.php",['a'=>$data]);

    will spew the warning 2000 times.

    & php post.php | grep Warning | wc -l
    2000

    -Rasmus
  • Laruence at Jan 5, 2012 at 4:13 am

    On Thu, Jan 5, 2012 at 5:56 AM, Rasmus Lerdorf wrote:
    On 01/04/2012 01:48 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 01:27 PM, Stas Malyshev wrote:
    Hi!
    Right, like I said in my previous message, if this is caught by
    display_start_errors, I am ok with it. We need the default/no php.ini
    file case to not leak information like this.
    Just checked - it does not display error if display_startup_errors if
    off, does display if it's on.
    Right, that seems ok. The other thing is that we need to clarify that it
    actually only limits the number of variables per nesting level. The
    current name and the description doesn't make that clear. You can still
    send 1M post vars in a single POST if you just nest them in a 1000x1000
    2d array. Of course, this is likely going to hit the post_max_size
    limit, although many sites that do file uploads will have cranked that
    way up.
    Oh, and a final issue to address.

    This code:

    for($data=[],$i=0; $i<=999; $i++) $data[$i] = range(0,1001);
    echo curl_post("http://localhost/index.php",['a'=>$data]);

    will spew the warning 2000 times.

    & php post.php | grep Warning | wc -l
    2000
    could you try with this new patch:
    https://bugs.php.net/patch-display.php?bug_id=60655&patch=max_input_vars.patch&revision=latest
    ?

    this patch also restrict the json / serializer , all of them must
    less than PG(max_input_vars).

    and different with the fix which was commited now, this patch count
    the num vars in a global scope, that means if there are 2 elements
    which both have 500 elements in post, the restriction will also
    affect,

    and this patch will not affect the existsing called to json or
    serializer, only affect the zif_json_decode and zif_serialize,
    after patch, the serialize will have a sencode parameter :"$max_vars".

    and the restriction can also be closed by set max_input_vars to 0.

    thanks
    -Rasmus

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Rasmus Lerdorf at Jan 5, 2012 at 4:50 am

    On 01/04/2012 08:13 PM, Laruence wrote:
    On Thu, Jan 5, 2012 at 5:56 AM, Rasmus Lerdorf wrote:
    On 01/04/2012 01:48 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 01:27 PM, Stas Malyshev wrote:
    Hi!
    Right, like I said in my previous message, if this is caught by
    display_start_errors, I am ok with it. We need the default/no php.ini
    file case to not leak information like this.
    Just checked - it does not display error if display_startup_errors if
    off, does display if it's on.
    Right, that seems ok. The other thing is that we need to clarify that it
    actually only limits the number of variables per nesting level. The
    current name and the description doesn't make that clear. You can still
    send 1M post vars in a single POST if you just nest them in a 1000x1000
    2d array. Of course, this is likely going to hit the post_max_size
    limit, although many sites that do file uploads will have cranked that
    way up.
    Oh, and a final issue to address.

    This code:

    for($data=[],$i=0; $i<=999; $i++) $data[$i] = range(0,1001);
    echo curl_post("http://localhost/index.php",['a'=>$data]);

    will spew the warning 2000 times.

    & php post.php | grep Warning | wc -l
    2000
    could you try with this new patch:
    https://bugs.php.net/patch-display.php?bug_id=60655&patch=max_input_vars.patch&revision=latest
    ?

    this patch also restrict the json / serializer , all of them must
    less than PG(max_input_vars).

    and different with the fix which was commited now, this patch count
    the num vars in a global scope, that means if there are 2 elements
    which both have 500 elements in post, the restriction will also
    affect,

    and this patch will not affect the existsing called to json or
    serializer, only affect the zif_json_decode and zif_serialize,
    after patch, the serialize will have a sencode parameter :"$max_vars".

    and the restriction can also be closed by set max_input_vars to 0.
    Right, I don't think this is going to work. A simple 'make install'
    after applying your patch fails with:

    Warning: unserialize(): Unserialized variables exceeded 1000 in
    phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
    on line 1145

    Warning: unserialize(): Unserialized variables exceeded 1000 in
    phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
    on line 1145

    Warning: unserialize(): Unserialized variables exceeded 1000 in
    phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
    on line 1145
    [PEAR] PEAR: pear.php.net/PEAR not installed

    I really don't think this manual per-feature limiting is going to cut it
    here. The real problem is the predictability of the hashing which we can
    address by seeding it with a random value. That part is easy enough, the
    hard part is figuring out where people may be storing these hashes
    externally and providing some way of associating the seed with these
    external caches so they will still work.

    -Rasmus
  • Laruence at Jan 5, 2012 at 5:02 am

    On Thu, Jan 5, 2012 at 12:50 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 08:13 PM, Laruence wrote:
    On Thu, Jan 5, 2012 at 5:56 AM, Rasmus Lerdorf wrote:
    On 01/04/2012 01:48 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 01:27 PM, Stas Malyshev wrote:
    Hi!
    Right, like I said in my previous message, if this is caught by
    display_start_errors, I am ok with it. We need the default/no php.ini
    file case to not leak information like this.
    Just checked - it does not display error if display_startup_errors if
    off, does display if it's on.
    Right, that seems ok. The other thing is that we need to clarify that it
    actually only limits the number of variables per nesting level. The
    current name and the description doesn't make that clear. You can still
    send 1M post vars in a single POST if you just nest them in a 1000x1000
    2d array. Of course, this is likely going to hit the post_max_size
    limit, although many sites that do file uploads will have cranked that
    way up.
    Oh, and a final issue to address.

    This code:

    for($data=[],$i=0; $i<=999; $i++) $data[$i] = range(0,1001);
    echo curl_post("http://localhost/index.php",['a'=>$data]);

    will spew the warning 2000 times.

    & php post.php | grep Warning | wc -l
    2000
    could you try with this new patch:
    https://bugs.php.net/patch-display.php?bug_id=60655&patch=max_input_vars.patch&revision=latest
    ?

    this patch also restrict the json / serializer ,  all of them must
    less than PG(max_input_vars).

    and different with the fix which was commited now,  this patch count
    the num vars in a global scope, that means if there are 2 elements
    which both have 500 elements in post,  the restriction will also
    affect,

    and this patch will not affect the existsing called to json or
    serializer,   only affect the zif_json_decode and zif_serialize,
    after patch, the serialize will have a sencode parameter :"$max_vars".

    and the restriction can also be closed by set max_input_vars to 0.
    Right, I don't think this is going to work. A simple 'make install'
    after applying your patch fails with:

    Warning: unserialize(): Unserialized variables exceeded 1000 in
    phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
    on line 1145

    Warning: unserialize(): Unserialized variables exceeded 1000 in
    phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
    on line 1145

    Warning: unserialize(): Unserialized variables exceeded 1000 in
    phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
    on line 1145
    [PEAR] PEAR: pear.php.net/PEAR not installed

    I really don't think this manual per-feature limiting is going to cut it
    here. The real problem is the predictability of the hashing which we can
    address by seeding it with a random value. That part is easy enough, the
    Hi:
    that will be a better fix, we have disscussed this before in irc, I
    also can have a try in that way.

    and for this fix(in such way), to be honest, yes, this is a little
    ugly fix, but quick.

    the default 1000 is a little insufficient when counting the elements
    in a global scope.

    IMO it should set to be 4096, then I think 99% of developers will
    not see this warning at all.

    thanks
    hard part is figuring out where people may be storing these hashes
    externally and providing some way of associating the seed with these
    external caches so they will still work.

    -Rasmus


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Stas Malyshev at Jan 5, 2012 at 7:09 am
    Hi!
    I really don't think this manual per-feature limiting is going to cut it
    here. The real problem is the predictability of the hashing which we can
    address by seeding it with a random value. That part is easy enough, the
    hard part is figuring out where people may be storing these hashes
    externally and providing some way of associating the seed with these
    external caches so they will still work.
    I think we'd need and API to access the seed value and to calculate hash
    for given seed value. That would probably allow extensions that store
    hashes with some additional work to do it properly. Though it needs more
    detailed investigation.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Rasmus Lerdorf at Jan 5, 2012 at 7:30 am

    On 01/04/2012 11:09 PM, Stas Malyshev wrote:
    Hi!
    I really don't think this manual per-feature limiting is going to cut it
    here. The real problem is the predictability of the hashing which we can
    address by seeding it with a random value. That part is easy enough, the
    hard part is figuring out where people may be storing these hashes
    externally and providing some way of associating the seed with these
    external caches so they will still work.
    I think we'd need and API to access the seed value and to calculate hash
    for given seed value. That would probably allow extensions that store
    hashes with some additional work to do it properly. Though it needs more
    detailed investigation.
    Yes, but we still need an actual case to look at. Opcode caches
    shouldn't be a problem unless they store some representation on disk
    that live across server restarts. In the APC world, nobody does that. Is
    there something in common use out there that actually needs this?

    Let's do just the GPC fix (the Dmitry version) for 5.3 and turn on
    ignore_repeated_errors just during startup and get it out there. That
    takes care of the most obvious attack vector for existing users. Leaving
    this in place for 5.4 is fine regardless of what we do in 5.4.

    I think for 5.4 we should take a couple of days to dig into what would
    actually break from seeding the hash. This seems like a much more
    elegant solution compared to trying to add limits to all the other
    places manually. This manual limit checking also wouldn't cover 3rd
    party extensions or even userspace code that might be vulnerable to the
    same thing. The only way to fix those cases is with a central hash fix.

    Another alternative to seeding would be to use a different hashing
    algorithm altogether. That would solve the cross-server issues, at the
    likely cost of slower hashing.

    -Rasmus
  • Stas Malyshev at Jan 5, 2012 at 7:33 am
    Hi!
    Yes, but we still need an actual case to look at. Opcode caches
    shouldn't be a problem unless they store some representation on disk
    that live across server restarts. In the APC world, nobody does that. Is
    It's not that simple. Take example of FastCGI setup where processes can
    live and die independently (note that FastCGI does not mandate
    single-parent fork and in fact on Windows it doesn't work that way). In
    this setup opcode cache is shared between processes which may have
    different hash value, unless we give means to the cache to set this
    value on startup somehow.

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Laruence at Jan 5, 2012 at 7:50 am
    Hi:
    there is one way maybe is a good try.

    when resize hashtable, we don't just dobule the size, instead, we
    increase the hashtable size with a random delta

    what do you think?

    thanks
    On Thu, Jan 5, 2012 at 3:33 PM, Stas Malyshev wrote:
    Hi!

    Yes, but we still need an actual case to look at. Opcode caches
    shouldn't be a problem unless they store some representation on disk
    that live across server restarts. In the APC world, nobody does that. Is

    It's not that simple. Take example of FastCGI setup where processes can live
    and die independently (note that FastCGI does not mandate single-parent fork
    and in fact on Windows it doesn't work that way). In this setup opcode cache
    is shared between processes which may have different hash value, unless we
    give means to the cache to set this value on startup somehow.


    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Rasmus Lerdorf at Jan 5, 2012 at 7:57 am

    On 01/04/2012 11:49 PM, Laruence wrote:
    Hi:
    there is one way maybe is a good try.

    when resize hashtable, we don't just dobule the size, instead, we
    increase the hashtable size with a random delta

    what do you think?
    Sorry, you lost me. How does that help? The problem is when we collide
    on a single hash key the resulting linked list traversion gets longer
    and longer as more colliding keys are added to that hashtable. Whether
    you double the size or grow it by some other factor doesn't change this.

    -Rasmus
  • Laruence at Jan 5, 2012 at 8:05 am

    On Thu, Jan 5, 2012 at 3:57 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 11:49 PM, Laruence wrote:
    Hi:
    there is one way  maybe is a good try.

    when resize hashtable,  we don't just dobule the size,  instead, we
    increase the hashtable size with a random delta

    what do you think?
    Sorry, you lost me. How does that help? The problem is when we collide
    on a single hash key the resulting linked list traversion gets longer
    and longer as more colliding keys are added to that hashtable. Whether
    you double the size or grow it by some other factor doesn't change this.
    No, No,

    if we increase the table size with a random delta(the nTableMask will
    also be random), then the collision will not be predictable.

    thanks
    -Rasmus


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Laruence at Jan 5, 2012 at 8:11 am

    On Thu, Jan 5, 2012 at 4:04 PM, Laruence wrote:
    On Thu, Jan 5, 2012 at 3:57 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 11:49 PM, Laruence wrote:
    Hi:
    there is one way  maybe is a good try.

    when resize hashtable,  we don't just dobule the size,  instead, we
    increase the hashtable size with a random delta

    what do you think?
    Sorry, you lost me. How does that help? The problem is when we collide
    on a single hash key the resulting linked list traversion gets longer
    and longer as more colliding keys are added to that hashtable. Whether
    you double the size or grow it by some other factor doesn't change this.
    No, No,

    if we increase the table size with a random delta(the nTableMask will
    also be random), then the collision will not be predictable.
    I have made a patch, plz try this mechanism,
    https://bugs.php.net/patch-display.php?bug_id=60655&patch=rand_hash_resize.patch&revision=latest

    I have tested , that works.

    and the patch is still a rough draft, so maybe some compile warnings..

    thanks
    thanks
    -Rasmus


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Laruence at Jan 5, 2012 at 8:24 am

    On Thu, Jan 5, 2012 at 4:11 PM, Laruence wrote:
    On Thu, Jan 5, 2012 at 4:04 PM, Laruence wrote:
    On Thu, Jan 5, 2012 at 3:57 PM, Rasmus Lerdorf wrote:
    On 01/04/2012 11:49 PM, Laruence wrote:
    Hi:
    there is one way  maybe is a good try.

    when resize hashtable,  we don't just dobule the size,  instead, we
    increase the hashtable size with a random delta

    what do you think?
    Sorry, you lost me. How does that help? The problem is when we collide
    on a single hash key the resulting linked list traversion gets longer
    and longer as more colliding keys are added to that hashtable. Whether
    you double the size or grow it by some other factor doesn't change this.
    No, No,

    if we increase the table size with a random delta(the nTableMask will
    also be random), then the collision will not be predictable.
    I have made a patch, plz try this mechanism,
    https://bugs.php.net/patch-display.php?bug_id=60655&patch=rand_hash_resize.patch&revision=latest

    I have tested , that works.

    and the patch is still a rough draft, so maybe some compile warnings..
    the key point is, increase the table size in a random delta (2 * size
    + (random_num & size_mask));

    in the same time use mod(%) instead of and(&) while doing the index mapping.

    then the attacker will be not able to predicate the collision ;)

    thanks.
    thanks
    thanks
    -Rasmus


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Rasmus Lerdorf at Jan 5, 2012 at 7:55 am

    On 01/04/2012 11:33 PM, Stas Malyshev wrote:
    Hi!
    Yes, but we still need an actual case to look at. Opcode caches
    shouldn't be a problem unless they store some representation on disk
    that live across server restarts. In the APC world, nobody does that. Is
    It's not that simple. Take example of FastCGI setup where processes can
    live and die independently (note that FastCGI does not mandate
    single-parent fork and in fact on Windows it doesn't work that way). In
    this setup opcode cache is shared between processes which may have
    different hash value, unless we give means to the cache to set this
    value on startup somehow.
    hrm, ok, yes for non single parent forks it is an issue. For APC there
    is no such thing, although people have been asking for it.

    With a clean API this doesn't sound insurmountable though.

    get_hash_seed()/set_hash_seed() internal functions. When the cache is
    created the first time call get_hash_seed() and store it somewhere. When
    someone connects to an existing cache the first time, fetch the stored
    hash seed and call set_hash_seed()

    -Rasmus
  • Derick Rethans at Jan 5, 2012 at 10:09 am

    On Wed, 4 Jan 2012, Rasmus Lerdorf wrote:
    On 01/04/2012 11:09 PM, Stas Malyshev wrote:

    I think we'd need and API to access the seed value and to calculate
    hash for given seed value. That would probably allow extensions that
    store hashes with some additional work to do it properly. Though it
    needs more detailed investigation.
    Yes, but we still need an actual case to look at. Opcode caches
    shouldn't be a problem unless they store some representation on disk
    that live across server restarts. In the APC world, nobody does that.
    Is there something in common use out there that actually needs this?
    bcompiler would need it

    cheers,
    Derick

    --
    http://derickrethans.nl | http://xdebug.org
    Like Xdebug? Consider a donation: http://xdebug.org/donate.php
    twitter: @derickr and @xdebug
  • Stas Malyshev at Jan 5, 2012 at 7:01 am
    Hi!
    and different with the fix which was commited now, this patch count
    the num vars in a global scope, that means if there are 2 elements
    which both have 500 elements in post, the restriction will also
    affect,
    Why? The point of the limitation is to avoid hash collisions and related
    performance problems, but if they are in different elements, what is the
    point of limiting them?

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Laruence at Jan 5, 2012 at 7:09 am

    On Thu, Jan 5, 2012 at 3:01 PM, Stas Malyshev wrote:
    Hi!

    and different with the fix which was commited now,  this patch count
    the num vars in a global scope, that means if there are 2 elements
    which both have 500 elements in post,  the restriction will also
    affect,

    Why? The point of the limitation is to avoid hash collisions and related
    performance problems, but if they are in different elements, what is the
    point of limiting them?
    Hi, this patch is aim at a quick/simple fix than before, that is why I
    proposal this patch.

    actually, there might be no attack even a array has more than 1000 elements,

    I mean, this is a simple / quick fix but works the same.

    thanks
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227


    --
    Laruence  Xinchen Hui
    http://www.laruence.com/
  • Reindl Harald at Jan 4, 2012 at 8:15 pm

    Am 04.01.2012 21:02, schrieb Rasmus Lerdorf:
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written.
    if it is well written there would be nor errors displayed
    but you miss - in production you MUST NOT dispaly errors
    They can check for potential errors and avoid them. This one can't be checked
    for and you could easily write a scanner that scoured the Net for sites
    with display_errors enabled by sending a relatively short POST request
    to each one and checking for this error.
    does not matter

    if display_errors is on DISPLAY it

    if it is off do NOT
    there is nothing between

    every try to make exceptions here is simply a bad style and should
    not be done - where do you stop? you can't decide - only the
    admin or developer with ini_set() has to decide and nobody else
  • Stas Malyshev at Jan 4, 2012 at 8:29 pm
    Hi!
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written. They
    Oh no. Nobody should or can safely run production with display_errors.
    Everybody thinks their code is well-written, but display_errors should
    never be enabled in production, however high is your opinion of the code.
    I'm afraid people now will start quoting this saying "ok, yeah, if
    you're a bad programmer, disable display_errors, but I'm a good
    programmer, my code is solid, I even have a dozen of unit tests, so I
    just go ahead and enable display_errors" and then we have this sad state
    of affairs where sites spill out error messages that are never supposed
    to be seen by clients because developers thought it can never happen.
    The alternative is to just not have any error message at all. That
    avoids the discrepancy between the error messages with display_errors on
    and off.
    I don't think it's a good idea to add such magic, as correctly noted, it
    will make people that are working properly - display off in production,
    on in development - work harder and have trouble, all in the name of
    cuddling people that run misconfigured servers and ignore the advice
    that is being repeated for years by now.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Johannes Schlüter at Jan 4, 2012 at 8:46 pm

    On Wed, 2012-01-04 at 12:29 -0800, Stas Malyshev wrote:
    Hi!
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written. They
    Oh no. Nobody should or can safely run production with display_errors.
    Everybody thinks their code is well-written, but display_errors should
    never be enabled in production, however high is your opinion of the code.
    I'm afraid people now will start quoting this saying "ok, yeah, if
    you're a bad programmer, disable display_errors, but I'm a good
    programmer, my code is solid, I even have a dozen of unit tests, so I
    just go ahead and enable display_errors" and then we have this sad state
    of affairs where sites spill out error messages that are never supposed
    to be seen by clients because developers thought it can never happen.
    On shared hosts display_errors typically is on, but the application can
    do ini_set('display_errors', 0) or such ...

    johannes
  • Jannik Zschiesche at Jan 4, 2012 at 8:49 pm
    Hi Johannes,

    as far as I understood the issue, this error would be triggered before the application's code is executed, so that would not solve this issue.



    Cheers
    Jannik

    Am 04.01.2012 um 21:46 schrieb Johannes Schlüter:
    On Wed, 2012-01-04 at 12:29 -0800, Stas Malyshev wrote:
    Hi!
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written. They
    Oh no. Nobody should or can safely run production with display_errors.
    Everybody thinks their code is well-written, but display_errors should
    never be enabled in production, however high is your opinion of the code.
    I'm afraid people now will start quoting this saying "ok, yeah, if
    you're a bad programmer, disable display_errors, but I'm a good
    programmer, my code is solid, I even have a dozen of unit tests, so I
    just go ahead and enable display_errors" and then we have this sad state
    of affairs where sites spill out error messages that are never supposed
    to be seen by clients because developers thought it can never happen.
    On shared hosts display_errors typically is on, but the application can
    do ini_set('display_errors', 0) or such ...

    johannes


    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
  • Johannes Schlüter at Jan 4, 2012 at 8:52 pm

    On Wed, 2012-01-04 at 21:49 +0100, Jannik Zschiesche wrote:
    Hi Johannes,

    as far as I understood the issue, this error would be triggered before
    the application's code is executed, so that would not solve this
    issue.
    That's the point. Thanks for making it clear. :-)

    johannes
    Cheers
    Jannik
  • Stas Malyshev at Jan 4, 2012 at 8:55 pm
    Hi!
    as far as I understood the issue, this error would be triggered before
    the application's code is executed, so that would not solve this
    issue.
    That's the point. Thanks for making it clear. :-)
    Hmm... shared hosts. OK, let's make it the same way as the other input
    message then. I don't like either of them but at least they'd do the
    same thing (and when somebody finally has an idea how to properly fix it
    it would be the same in both places).
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Jannik Zschiesche at Jan 4, 2012 at 8:58 pm
    Hi again,


    Am 04.01.2012 um 21:52 schrieb Johannes Schlüter:
    On Wed, 2012-01-04 at 21:49 +0100, Jannik Zschiesche wrote:
    Hi Johannes,

    as far as I understood the issue, this error would be triggered before
    the application's code is executed, so that would not solve this
    issue.
    That's the point. Thanks for making it clear. :-)

    what could work nevertheless (if the server is an apache and htaccess is enabled) is setting display_errors to off with `php_flag display_errors off`, I guess.



    Cheers
    Jannik
    johannes
    Cheers
    Jannik

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
  • Rasmus Lerdorf at Jan 4, 2012 at 8:59 pm

    On 01/04/2012 12:46 PM, Johannes Schlüter wrote:
    On Wed, 2012-01-04 at 12:29 -0800, Stas Malyshev wrote:
    Hi!
    But there is a very valid security concern here. People can usually run
    safely with display_errors enabled if their code is well-written. They
    Oh no. Nobody should or can safely run production with display_errors.
    Everybody thinks their code is well-written, but display_errors should
    never be enabled in production, however high is your opinion of the code.
    I'm afraid people now will start quoting this saying "ok, yeah, if
    you're a bad programmer, disable display_errors, but I'm a good
    programmer, my code is solid, I even have a dozen of unit tests, so I
    just go ahead and enable display_errors" and then we have this sad state
    of affairs where sites spill out error messages that are never supposed
    to be seen by clients because developers thought it can never happen.
    On shared hosts display_errors typically is on, but the application can
    do ini_set('display_errors', 0) or such ...
    But that is precisely why this is a special case. Even if you do
    ini_set('display_errors', 0) in your code this message will still be
    displayed. Although, display_startup_errors is off by default and
    hopefully this one falls under that setting. I didn't test it, but if it
    doesn't then we need to make sure it is suppressed when
    display_startup_errors is off.

    -Rasmus

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedJan 4, '12 at 7:00a
activeJan 5, '12 at 10:09a
posts49
users13
websitephp.net

People

Translate

site design / logo © 2022 Grokbase