[RFC] is_literal()

On Sat, Mar 21, 2020, at 2:13 PM, Craig Francis wrote:

While I appreciate the intent, without an untaint() or equivalent I fear its usefulness will be limited, or else it will get overused and thus cut off numerous entirely valid situations. Eg, there's plenty of very good reasons to put a template string into the database rather than a file literal. Or to build an SQL query dynamically in ways that an is_literal check would not allow, at least not without an absurdly complex query builder. Without a way to flag "yes, I know this was built dynamically but I've vetted it, it's OK" on a value, I fear such a check will either be unuseful or counter-productive. --Larry Garfield

On Sat, 21 Mar 2020 at 19:14, Craig Francis <craig@craigfrancis.co.uk> wrote:

The RFC should probably define what a literal is. cheers Dan Ack

This seems very similar to the taint extension. How does it differ from ext-taint, and have you considered working together with ext-taint? https://www.php.net/manual/en/intro.taint.php The name is_literal() is too ambiguous to me. As Dan suggested, the RFC should define what a literal is. Cheers, Ben

Hi Craig, https://github.com/laruence/taint#taint notes that "Please note that do not enable this extension in product(ion) env, since it will slowdown your app." - That repo already provides is_tainted() http://docs.php.net/is_tainted A fork of that repo would theoretically allow implementing is_literal() as described in the RFC - is that the implementation plan? - The slowdown would be a large problem if this feature was always on. And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php. If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future. If it's implemented in the same way as taint (i.e. cannot be used in combination with XDebug, blackfire, newrelic, etc), that would also be a problem for including it in core. If it wasn't, then it'd slow down concatenation, calls, etc. even when the application didn't need is_literal. I also imagine that whether or not opcache was enabled is likely to affect whether or not something ends up being a literal or not (e.g. opcache can evaluate functions such as str_repeat() for literals at compile time) https://github.com/laruence/taint/blob/master/taint.c seems to already support a whitelist (php_taint_override_func), so that isn't insurmountable for functions, but it's possible `if ($local === 'literal') { process($local); }` would only satisfy is_literal() with opcache enabled. Related projects (static analysis instead of runtime checks, though): It's also worth noting that `vimeo/psalm` had an in progress way to detect some ways in which tainted strings may be used by applications, based on a paper by Facebook. (https://cacm.acm.org/magazines/2019/8/238344-scaling-static-analyses-at-facebook/fulltext (for HHVM, though)) https://github.com/vimeo/psalm/issues/611#issuecomment-515153305 - but it isn't completed or usable yet, as far as I can tell. Wikimedia also created https://gerrit.wikimedia.org/g/mediawiki/tools/phan/SecurityCheckPlugin/ , but that's currently beta. Both would have ways they fail to catch every way an argument could be passed to a function (e.g. unanalyzable dynamic/framework calls) - Tyson