5 ms·

I think the patch should have been to rip out eval() and build/use a shunting-yard parser/evaluator. A quick search found this: https://github.com/ircmaxell/ph

I think the patch should have been to rip out eval() and build/use a shunting-yard parser/evaluator. A quick search found this: https://github.com/ircmaxell/php-math-parser https://github.com/ircmaxell/php-math-parser Leaving a call to eval() with user supplied input, no matter how well filtered, seems like way too big of a risk.

ihumanable 8y ago
Agreed, when you are on patch #4 because the previous patches weren't effective, and you can't verify patch #4 because of time constraints, it might be time to step back and reconsider the approach. Sounds like they should just not be eval()ing user input regardless of how much sanitization they throw at it.
sannee 8y ago
Or use whatever tools are included in their language for safely evaluating strings (like ast.literal_eval in Python).
- Something1234 8y ago
  It's PHP there is none. In fact most languages don't include a safe way to evaluate strings out of the box.
  - jsjohnst 8y ago
    > It's PHP there is none. This seems to disprove that, no? https://secure.php.net/manual/en/runkit.sandbox.php https://secure.php.net/manual/en/runkit.sandbox.php
    - Something1234 8y ago
      Allow me to rephrase, no cheap way to evaluate arbitrary strings without spinning up another vm.
      - jsjohnst 8y ago
        That’s not true either.
        workinthehead 8y ago
        explain?
    - workinthehead 8y ago
      That's an extension. It doesn't come with PHP (not "out of the box").
      - jsjohnst 8y ago
        Rails doesn’t come with a default install of Ruby. Xcode doesn’t come pre-installed on macOS. You have to install Chrome on every major desktop OS as it doesn’t come by default. Installing and configuring your environment to your needs should be entry level stuff for a developer, so what was the point of your reply?
        workinthehead 8y ago
        Can you explain why PHP developers are so uniquely unable to accept honest criticism of their language? Like, whence all the butthurt, bro? The original point was that PHP doesn't have a safe string eval built-in. Which is completely fucking true. Rage all you want. Moreover, most PHP devs (probably rightfully) won't install runkit because of all the other clowny shit it enables, but that's an entirely other kettle of fish.
        jsjohnst 8y ago
        > Can you explain why PHP developers are so uniquely unable to accept honest criticism of their language? Like, whence all the butthurt, bro? I’m not a PHP developer. Nice try to deflect though! You obvious have an agenda you are trying to push at all costs and don’t want to learn, so have a great day!
        deleted 8y ago
        [deleted]
        workinthehead 8y ago
        Jeremy, your Github says otherwise: https://github.com/jsjohnst https://github.com/jsjohnst Why you gotta lie, man? You've completely made an ass of yourself.
userbinator 8y ago
IMHO precedence climbing (a refactored version of recursive descent) would be much simpler and makes parsing seem almost trivial, since it can be made table-driven: https://www.engr.mun.ca/~theo/Misc/exp_parsing.htm https://www.engr.mun.ca/~theo/Misc/exp_parsing.htm Speaking of parsing simplicity and software design in general, you've coincidentally found a great example of how not to (ab)use OOP... one line of actual work wrapped in a dozen lines of boilerplate and replicated across multiple files with tiny variations: https://github.com/ircmaxell/php-math-parser/tree/master/lib/PHPMathParser/Expressions https://github.com/ircmaxell/php-math-parser/tree/master/lib... To put things into perspective, a precedence-climber for 13 levels of precedence from C's grammar, including a tokeniser, fits comfortably in <300 LoC of C, the bulk of that being the tokeniser. PHP's higher level nature (especially around string handling) would probably make a corresponding implementation even smaller.

ihumanable 8y ago

Agreed, when you are on patch #4 because the previous patches weren't effective, and you can't verify patch #4 because of time constraints, it might be time to step back and reconsider the approach. Sounds like they should just not be eval()ing user input regardless of how much sanitization they throw at it.

sannee 8y ago

Or use whatever tools are included in their language for safely evaluating strings (like ast.literal_eval in Python).

Something1234 8y ago
It's PHP there is none. In fact most languages don't include a safe way to evaluate strings out of the box.
- jsjohnst 8y ago
  > It's PHP there is none. This seems to disprove that, no? https://secure.php.net/manual/en/runkit.sandbox.php https://secure.php.net/manual/en/runkit.sandbox.php
  - Something1234 8y ago
    Allow me to rephrase, no cheap way to evaluate arbitrary strings without spinning up another vm.
    - jsjohnst 8y ago
      That’s not true either.
      - workinthehead 8y ago
        explain?
  - workinthehead 8y ago
    That's an extension. It doesn't come with PHP (not "out of the box").
    - jsjohnst 8y ago
      Rails doesn’t come with a default install of Ruby. Xcode doesn’t come pre-installed on macOS. You have to install Chrome on every major desktop OS as it doesn’t come by default. Installing and configuring your environment to your needs should be entry level stuff for a developer, so what was the point of your reply?
      - workinthehead 8y ago
        Can you explain why PHP developers are so uniquely unable to accept honest criticism of their language? Like, whence all the butthurt, bro? The original point was that PHP doesn't have a safe string eval built-in. Which is completely fucking true. Rage all you want. Moreover, most PHP devs (probably rightfully) won't install runkit because of all the other clowny shit it enables, but that's an entirely other kettle of fish.
        jsjohnst 8y ago
        > Can you explain why PHP developers are so uniquely unable to accept honest criticism of their language? Like, whence all the butthurt, bro? I’m not a PHP developer. Nice try to deflect though! You obvious have an agenda you are trying to push at all costs and don’t want to learn, so have a great day!
        deleted 8y ago
        [deleted]
        workinthehead 8y ago
        Jeremy, your Github says otherwise: https://github.com/jsjohnst https://github.com/jsjohnst Why you gotta lie, man? You've completely made an ass of yourself.

userbinator 8y ago

IMHO precedence climbing (a refactored version of recursive descent) would be much simpler and makes parsing seem almost trivial, since it can be made table-driven: https://www.engr.mun.ca/~theo/Misc/exp_parsing.htm https://www.engr.mun.ca/~theo/Misc/exp_parsing.htm Speaking of parsing simplicity and software design in general, you've coincidentally found a great example of how not to (ab)use OOP... one line of actual work wrapped in a dozen lines of boilerplate and replicated across multiple files with tiny variations: https://github.com/ircmaxell/php-math-parser/tree/master/lib/PHPMathParser/Expressions https://github.com/ircmaxell/php-math-parser/tree/master/lib... To put things into perspective, a precedence-climber for 13 levels of precedence from C's grammar, including a tokeniser, fits comfortably in <300 LoC of C, the bulk of that being the tokeniser. PHP's higher level nature (especially around string handling) would probably make a corresponding implementation even smaller.