Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114560
MIME-Version: 1.0
References: <CAFv4g+EK_5YF+YGdKBYfYX-_730k39Fdq4hAG4gEApebyv5eDg@mail.gmail.com>
 <CAFv4g+Emp3L=w6tMG9ZLMh3a8Bb0V_aSrD9A-TRLwNn1ODNkug@mail.gmail.com>
In-Reply-To: <CAFv4g+Emp3L=w6tMG9ZLMh3a8Bb0V_aSrD9A-TRLwNn1ODNkug@mail.gmail.com>
Date: Fri, 21 May 2021 22:21:31 +0100
Message-ID: <CAFv4g+E_dfErbkovpu2YxcGrzyKNdOKzee6TfbrfCXgqEs3NCg@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000006df2e305c2dda5f4"
Subject: Re: Injection vulnerabilities
From: craig@craigfrancis.co.uk (Craig Francis)

--0000000000006df2e305c2dda5f4
Content-Type: text/plain; charset="UTF-8"

Fine, I'll finish with this...

We know that HTML [3] and SQL [4] should be written by the programmer, with
user data being handled *separately*.

The same applies to OS Commands:

  $command = 'rm -rf ?';

Because we're using parameters (to escape the user values properly), we
don't need to consider injection vulnerabilities (yay).

But we must NEVER say these strings, written by a programmer, are "safe".

For example, while the use of parameters in $command does avoid an
injection vulnerability, it's still a big problem if a path to something
that shouldn't be deleted is used (insert classic `rm -rf /` joke here).

So we cannot say anything is "safe", but we can note something valuable
about a variable - was it written by the programmer.

Or, in other words, was this variable created from a literal (a string
defined in the PHP script).

---

Now you might say that we should use static analysis to identify
mistakes... yeah, most programmers do not (it's an extra step they can't be
bothered to do)... and even then, Psalm and PHPStan currently focus on
other issues (type checking, basic logic flaws, code formatting, etc)...
that said, Psalm does have a Taint Checking feature (run separately, and I
bet you're not using it), but Taint Checking is also flawed:

  $sql = 'SELECT * FROM users WHERE id = ' . $db->real_escape_string($id);

  $html = "<img src=" . htmlentities($url) . " alt='' />";

  $html = "<a href='" . htmlentities($url) . "'>...";

The first two need the values to be quoted, but would be considered
"untained".

The third example, well htmlentities, before PHP 8.1 (thank you very much)
does not escape single quotes by default... not does it consider
'javascript:' URLs.

We need something that libraries will (in the future) be able to use to
protect themselves against these mistakes... by all programmers, especially
those who aren't using static analysis.

Craig

--0000000000006df2e305c2dda5f4--