On Sun, 22 Mar 2020 at 19:11, Mike Schinkel <m...@newclarity.net> wrote: > I think it would be better to specify the problem(s) you are trying to solve
Thanks for your thoughts Mike, I've updated the RFC, tweaking the definition of a literal, and moving that to the end of the introduction, so the problems are now in the second paragraph (sorry this wasn't clear, this is my first RFC). https://wiki.php.net/rfc/is_literal <https://wiki.php.net/rfc/is_literal> > Looking at my code [...] I pass to $wpdb->get_results() is in variables, not > literals. I think this is where Jakob's suggestion might help, to call it `is_from_literal()`. Because you're right, the SQL/HTML/Command is often created by one or more literals that get stored in a variable, and this proposal works with that approach. > [...] introduce functionality that addresses the exact problem of SQL > injection I'm fairly confident this proposal does that, and more. By allowing frameworks/developers to check the SQL/HTML/Command is made up of hard coded literals (from within the PHP script), it proves it's not vulnerable to any kind of injection, as the other variables/parameters must be provided separately. The examples in the RFC cover the issues that usually get raised when discussing this. But I do need to address the performance question (ideally with some help from someone who understands the PHP Internals, as I'm not experienced enough in that area). > [...] hash out potential solutions on the list rather than propose a specific > one in advance. I must confess, I have been discussing this for a number of years, and I've looked at a few different approaches, Taint checking got the closest, and this proposal takes that idea a bit further to resolve the last few issues (covered in the RFC, so I won't repeat them here). I've also been keeping this proposal in mind over the last couple of years, just to see how it would effect my development practices (and I really think it has helped). As to your idea of a "safe" MySQL class, fortunately mysqli already stops multiple queries, so a SELECT cannot have an UPDATE/DELETE/TRUNCATE appended on to the end, but it can still do things like UNION another SELECT query, so the original query returns nothing, then the attackers query gets appended, potentially allowing them to extract everything from the database. Craig On 22 Mar 2020, at 19:10, Mike Schinkel <m...@newclarity.net> wrote: > BTW, I did not comment on your is_literal() proposal because I try not to > comment on things where others are addressing concerns and where I do not > feel very strongly about adding or avoiding the feature. I created this > thread as a new thread to expliclty separate discussions because they are > orthogonal and I did not want to imply that I supported is_literal() as > currently proposed. > > But since you brought it up let me comment on is_literal(). I recognize the > problem I think you are trying to solve — to be able to guard against SQL > injection attacks — but I don't think is_literal() is a viable solution for > that problem. > > 1. Looking at my code most of my dynamic values use to compose SQL that I > pass to $wpdb->get_results() is in variables, not literals. In fact I rarely > use literals. So I don't see that it would help me (or many others?) much at > all. > > 2. Focusing on it being literal or not seems to me to be focusing on wrong > thing. Something could be non-literal but still be safe. And a code hack > could introduce a tainted literal (but with a code hack all bets are off.) > is_taint() makes more sense to me, but even then I am not sure it directly > addresses the use-case. > > 3. I feel like we might be better to introduce functionality that addresses > the exact problem of SQL injection and not one that dances around its edges. > Maybe we need a MySQL class as an alternative to the mysqli functions that > has a "safe" property and when that "safe" property is true you can't run > DDL, TRUNCATE, or DELETE? Not sure how to protect against injection for > UPDATE but maybe someone else has an idea? > > 4. Lastly, I think it would be better to specify the problem(s) you are > trying to solve and then hash out potential solutions on the list rather than > propose a specific one in advance, maybe even creating an ad-hoc working > group to come up with a solution that is likely to be accepted? > > Anyway, #jmtcw on is_literal(). > > -Mike
