On Wed, Jun 23, 2021 at 10:54 AM Craig Francis <cr...@craigfrancis.co.uk> wrote: > > On Wed, 23 Jun 2021 at 14:37, Larry Garfield <la...@garfieldtech.com> wrote: > > > I'm still very torn on is_literal; I fear that the people who would > > benefit from it are the very people that don't use the tools that would > > leverage it (DBALs et al), and so the net benefit will be small. > > > > > This RFC will not help those who aren’t using libraries (DBALs), that’s > something we could look at in the future (I have a suggestion in the Future > Scope section, but whatever that involves, it will need to use this flag, > so it would need to be in place first). > > But - and why I’m here - my experience is that it’s still a big issue for > those who *do* use libraries. It frequently comes up at software > agencies/companies that maintain their code (built with free > libraries/frameworks), and employ junior developers (i.e. the cheapest), > who make many "quick edits" (time is money), and in doing so introduce the > issues the RFC covers (and not to say we more experienced coders don’t > occasionally make mistakes too!). While non-library users are the main > cause, the library users are still a big part of why Injection > Vulnerabilities remain at the top of the OWASP Top 10. > > Craig
Hi, I was asked for my thoughts on this RFC (and its naming) from a security engineering perspective. The old is_literal() isn't correct if it includes non-string values (i.e. integers). The new is_trusted() is potentially misleading, especially to people who don't read the docs. My knee-jerk reaction was simply, "Why not is_untainted()?" but that invokes the imagery of taint-checking, which the RFC explicitly doesn't implement. A better name might be is_noble(), where we get to define the concept of noble inputs (name inspired by the Noble gases from Chemistry). The main reason I don't like is_trusted() is that everyone's threat model and risk tolerance is different, and trust is too nebulous a concept for a built-in function. But also, it's really easy to jump the guard-rail: https://3v4l.org/4GM8Q#focus=rfc.literals One concern that Joe Watkins asked about is: Is it reasonable to cover both integers and strings that are not influenceable by potential external attackers (n.b. ones that can't already overwrite your source code)? Outside the chr()/pack()/sprintf()/etc. technique demonstrated above, there's no risk of injection inherent to concatenating a trusted string with an untrusted integer. Injection attacks (SQL injection, LDAP injection, XSS, etc.) are, at their core, an instance of type confusion between data and code. In order for the injection to *do* anything, it needs to be in the same input domain as the language the code is written in. Try as you might, there is no integer that will, upon concatenation with a string, produce a control character for HTML (i.e. `>`) or SQL (i.e. `'`). Therefore, if the concern is Injection attacks, integer inputs do not need to be tracked to provide a security gain. This is only true for integers, not all numeric types. I haven't investigated the safety of floats in every possible context, and the `e`, `+`, and `.` characters could be problematic in corner cases. TL;DR - Why not is_noble()? - String + int concatenation isn't an injection risk. Cheers, Scott -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
