Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:115093 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 26540 invoked from network); 24 Jun 2021 05:51:42 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 24 Jun 2021 05:51:42 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 295D71804CC for ; Wed, 23 Jun 2021 23:10:23 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,HTML_MESSAGE, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail1.25mail.st (mail1.25mail.st [206.123.115.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 23 Jun 2021 23:10:21 -0700 (PDT) Received: from smtpclient.apple (unknown [49.48.221.121]) by mail1.25mail.st (Postfix) with ESMTPSA id A7F8E60328; Thu, 24 Jun 2021 06:10:08 +0000 (UTC) Message-ID: <95D16F2E-E9DD-4964-A0E2-62E1FB0D976B@koalephant.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_914C1CA4-73E2-4BE8-8C87-1E726491125A" Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.100.0.2.22\)) Date: Thu, 24 Jun 2021 13:10:03 +0700 In-Reply-To: Cc: Bruce Weirdan , Craig Francis , Larry Garfield , php internals To: Scott Arciszewski References: <03f7955c-69a8-4841-9245-449d7851e207@www.fastmail.com> X-Mailer: Apple Mail (2.3654.100.0.2.22) Subject: Re: [PHP-DEV] [RFC] Name issue - is_literal/is_trusted From: php-lists@koalephant.com (Stephen Reay) --Apple-Mail=_914C1CA4-73E2-4BE8-8C87-1E726491125A Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 24 Jun 2021, at 08:30, Scott Arciszewski = wrote: >=20 > On Wed, Jun 23, 2021, 9:23 PM Bruce Weirdan > wrote: >=20 >> On Thu, Jun 24, 2021 at 3:41 AM Scott Arciszewski = >> wrote: >>> The failure condition of this query is >>> "return all rows from the table already being queried", not "return >>> arbitrary data the attacker selects from any table that the >>> application can read". >>=20 >> Imagine that was a DELETE rather than SELECT and the failure = condition >> becomes 'the table is emptied'. >> It may have less disastrous consequences (depending on how good your >> backup / restore procedures are) compared to arbitrary reads you >> demonstrated, but it is still, quite clearly, a glaring security hole >> caused by user input in SQL query - AKA SQL injection in layman's >> terms. >>=20 >>> it differs from Injection vulnerabilities in one >>> fundamental way: The attacker cannot change the structure of the SQL >>> query being executed. >>=20 >> I would say replacing a column name with value is changing the >> structure of SQL query, and, basically, in exactly the way you >> describe SQL injection: confusing the code (column name) with data. >>=20 >> I wholeheartedly welcome this RFC as it was originally proposed: >> is_literal() doing exactly what it says on the tin, without any >> security claims. But it has gone far from there real quick and now >> people can't even name the thing. >>=20 >>=20 >> -- >> Best regards, >> Bruce Weirdan mailto: >> weirdan@gmail.com >=20 >=20 >=20 > We can agree that it is a bug. We don't agree on the definition of SQL > injection. >=20 > Changing a column name to a number (which prepared statements = shouldn't > allow in the first place) is a bug. This changes the effect of the = command, > but the *structure* of the query remains unchanged. Hi Scott, I wrote that example where an integer could be dangerous. So firstly - just to clarify, because some replies seemed to be confused = on the topic, as was literally mentioned in the original comment, it is = definitely not correct behaviour - it=E2=80=99s a developer mistake that = might work some of the time, and thus go unnoticed in testing. If you = can show me a developer who=E2=80=99s never inadvertently passed the = wrong parameter in some condition, I=E2=80=99ll show you an imaginary = developer. Additionally - pointing out that this is a "developer error=E2=80=9D = doesn=E2=80=99t help your case. Using non-parameterised queries should = already be a =E2=80=9Cdeveloper error=E2=80=9D for anyone who can walk = and breathe at the same time - and yet that usage is being actively = encouraged if this function supports integers. I=E2=80=99ve still seen = zero responses about legitimate reasons this needs to support integers - = giving people a shitty way to build an IN() clause is not legitimate. = Parameterisation exists, and works. I don=E2=80=99t even understand why you mentioned prepared statements (I = guess you meant using parameterised queries?) - the column name = inherently can=E2=80=99t be parameterised - hence having to use a string = substitution in the query.=20 That part was weird and confusing, but not as odd as your claim that = altering the query, so that it causes the where clause to become moot, = is not an SQL Injection? REALLY? That=E2=80=99s your claim? I did a little research, and it turns out Wikipedia = (https://en.wikipedia.org/wiki/SQL_injection#Technical_implementations = ), Cloudflare = (https://www.cloudflare.com/en-au/learning/security/threats/sql-injection/= = ), and OWASP = (https://owasp.org/www-community/attacks/SQL_Injection#example-2 = ) all have = examples with a `1=3D1` type query manipulation. Do you want to write = and tell them that they=E2=80=99re all wrong, or should I ask them to = call you? Also, while researching the specifics of what is considered an =E2=80=9CSQ= L Injection=E2=80=9D I came across an article, that talks specifically = about the dangers of allowing user input (i.e. the thing `is_trusted` is = meant to prevent) as a column or table identifier. It=E2=80=99s from = this little organisation, you may have heard of them: =E2=80=9CParagon = Initiative=E2=80=9D = (https://paragonie.com/blog/2015/05/preventing-sql-injection-in-php-applic= ations-easy-and-definitive-guide = ). I would absolutely make use of a function that tells me if the string = given is in fact from something controlled by the developer. But once = that same string can also include input from the request or the = environment or whatever by nature of integers, the function becomes = useless for the stated purpose. Cheers Stephen --Apple-Mail=_914C1CA4-73E2-4BE8-8C87-1E726491125A--