Re: [RFC] is_trusted - was is_literal

From: Date: Tue, 22 Jun 2021 14:05:03 +0000
Subject: Re: [RFC] is_trusted - was is_literal
References: 1 2 3 4 5 6 7  Groups: php.internals 
Request: Send a blank email to internals+get-115026@lists.php.net to get a copy of this message

> On 22 Jun 2021, at 20:13, Craig Francis <craig@craigfrancis.co.uk> wrote:
> 
> On Tue, 22 Jun 2021 at 09:59, Stephen Reay <php-lists@koalephant.com <mailto:php-lists@koalephant.com>> wrote:
> So I just want to make sure I understand the progression on this so far. It started out with
> people wanting a way to check that a string was a literal string, in code somewhere, and does not
> come from user input. Ok makes sense. The name makes sense too.
> 
> 
> 
> The primary reason was never just to define literal strings, the intention has always been to
> create a practical, implementable solution to address the issue of Injection Vulnerabilities
> (SQl/HTML/CLI/etc).
> 

Preventing injection vulnerabilities may be your goal but I’m talking about the intended behaviour
of this one function. Your original email says this:

>> Distinguishing strings from a trusted developer from strings that may be attacker
>> controlled



If you feel that somehow doesn’t mean the same as "check that a string was a literal string,
in code somewhere, and does not come from user input”, then we need to crack open a dictionary and
work out which words one of us doesn’t know the meaning of.




> The name is_literal() has always just been a placeholder, it came up when I first
> started looking at this problem because that was the most obvious thing I knew we could anchor
> around. (Unfortunately I think it was easy to make assumptions based solely on that name, rather
> than focussing on the issue it is meant to address).
> 
> So, we cannot look for literals only - while it was part of the solution, it was very much
> incomplete. Bearing in mind, there is considerable amount of existing code and tutorials out there
> which include integers in their SQL/HTML/CLI/etc, and they are perfectly safe in doing so. Making a
> solution which does not support integers is not going to be adopted/used because the task of
> rewriting and changing everything, for no benefit, will not be considered by developers.
> 

There is a considerable amount of existing code that includes strings in SQL, HTML without danger
too. Plenty of string values are fine, and plenty of integer values are fine. That doesn’t mean we
should just blindly trust a value that came from the user, just because it’s a number.
The saying goes “never trust user input” not “never trust user input unless it’s a
number”. 


> Likewise, a lot of code already builds SQL/HTML/CLI/etc strings via concatenation and
> sprintf(), and forcing everyone to use a query builder is likely to cause most people to not even
> consider using this.
> 

If they won’t adopt an existing solution to the problem why would they adopt this?
You’ve said very recently that this is not intended to be used directly by most developers, and
instead used within libraries and frameworks. It seems a little weird to then make concessions that
will defeat the stated goal, in the name of adoption. 


> It's all well thinking of one thing that might theoretically/idealistically solve the
> issue, but it also needs to have a plan on how this will be practically implemented and used by
> developers (which this has done).
>  


Having a plan for how to implement something doesn’t help much when the thing you’re
implementing deliberately ignores a specific type of ‘untrusted’ input..






Thread (24 messages)

« previous php.internals (#115026) next »