Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114540
MIME-Version: 1.0
Date: Thu, 20 May 2021 18:12:02 +0100
Message-ID: <CAFv4g+EK_5YF+YGdKBYfYX-_730k39Fdq4hAG4gEApebyv5eDg@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000007dddee05c2c60b07"
Subject: Injection vulnerabilities
From: craig@craigfrancis.co.uk (Craig Francis)

--0000000000007dddee05c2c60b07
Content-Type: text/plain; charset="UTF-8"

Hi Internals,

Just something for you to think about, to start the conversation (as the
8.1 deadline is fast approaching).

Injection Vulnerabilities remain on the OWASP Top 10 List, same with XSS.

https://owasp.org/www-project-top-ten/2017/

These mistakes happen when user supplied values are included in command
strings - be that SQL, HTML, OS Commands (e.g. shell_exec), and dare I say
it eval().

It's easy to make these mistakes, especially when using a library (API)
that hides the implementation details.

I've got loads of examples (one of the joys of auditing), but have a think
about these 3 for now...

---

*1*, Let's consider an ORM (I'm using CakePHP this time). Parts of the
where() array must be defined by the programmer. Both of these examples
work, but the first one has a simple mistake that has introduced an SQL
Injection vulnerability:

$articles->find()->where(['id != ' . $_GET['id']]);

$articles->find()->where(['id != ' => $_GET['id']]);

https://book.cakephp.org/3/en/orm/query-builder.html

---

*2*, Maybe the programmer is writing the SQL themselves, and passes it on
to a basic database abstraction; but they don't realise the SQL string
should be entirely written by the programmer, using parameterised queries*:

<input type="checkbox" name="select[]" value="1" />
<input type="checkbox" name="select[]" value="3" />

$sql = 'SELECT * FROM table WHERE id IN (' . implode(',', $_POST['select'])
. ')';

* Yes, I know, things like table names cannot use parameters, but they have
to be handled carefully as well, and will be covered.

---

*3*, How about a small HTML snippet, which should also be written entirely
by the programmer:

$html = "<img src=" . htmlentities($url) . " alt='' />";

Because escaping is hard (and sometimes completely forgotten), user values
should be added to HTML via a context-aware Templating Engine/Library (so
every one of the programmers using that well tested library don't have to
worry about making these mistakes all the time).

In this case, the src attribute value isn't quoted, and that kinda works,
but it can be exploited with $url = '/ onerror=alert(1)';

---

Just have a think about these issues, and note how they are all affected by
the same problem.

Craig

--0000000000007dddee05c2c60b07--