Joomla Extensions Development

Write code which doesn't suck
Prev	Chapter 5. General advice and code magic	Next

Write code which doesn't suck

Having audited several dozens of sites with hundreds of extensions I have seen the same old problems appearing in the code of extension after extension. Not only they are security incidents waiting to happen, they also make migrating your code to newer Joomla versions harder and break your extension in said Joomla versions.

DON'T: Hard-coded database queries

I have seen bad code like this:

$query = 'SELECT `something` FROM `#__whatever` WHERE `id` = ' . $db->quote($id);

It's even worse with there is no $db->quote at all because that ensures it's a SQL injection waiting to happen.

Having a hard-coded database query is really bad because you are making an assumption the site runs under MySQL and that the escape character is the backtick. This is not a guarantee. Moreover, are you absolutely sure that you have not missed an escape in a long SQL query? Would you bet your and your clients' sites' security, as well as the possibility of massive fines due to GDPR if you have failed miserably, on your ability to not miss anything?

Let's get real. Joomla has offered a database query builders since version 1.6 (released in 2010) and improved it in Joomla 4.

Here's the simplest way to write the above code:

$query = $db->getQuery(true)
    ->select($db->quoteName('something'))
    ->from($db->quoteName('#__whatever'))
    ->where($db->quoteName('id') . ' = ' . $db->quote($id));

This is still not a very good approach because $id may look like a numeric string but actually be a hidden SQL Injection. Therefore a better way to write this query is using prepared statements which send the raw value of $id to the server and tell it what data type it is (boolean, null, blob, string or integer). The value is not interpolated into the query itself, making it perfectly safe against SQL injection attacks.

$query = $db->getQuery(true)
    ->select($db->quoteName('something'))
    ->from($db->quoteName('#__whatever'))
    ->where($db->quoteName('id') . ' = :id')
    ->bind(':id', $id, \Joomla\Database\ParameterType::INTEGER);

Note

	Note
Prepared statements are only meant to be used when you have data coming from a variable. If you have hard-coded data you can still interpolate them after passing them through `$db->quote()`. Also note that any data —even data coming from another query which ostensibly only includes “safe”, hard-coded data you have inserted yourself in the database— MUST go through prepared statements. Anything which is not hard-coded in the code currently executing must be assumed to be potentially dangerous. Even data you inserted in the database and the user cannot modify through the interface can be modified by a malicious actor, e.g. using a SQL injection vulnerability in another extension. TRUST NO-ONE, NOT EVEN YOURSELF. That's the motto of the security conscious developer.

Prepared statements are only meant to be used when you have data coming from a variable. If you have hard-coded data you can still interpolate them after passing them through $db->quote().

Also note that any data —even data coming from another query which ostensibly only includes “safe”, hard-coded data you have inserted yourself in the database— MUST go through prepared statements. Anything which is not hard-coded in the code currently executing must be assumed to be potentially dangerous. Even data you inserted in the database and the user cannot modify through the interface can be modified by a malicious actor, e.g. using a SQL injection vulnerability in another extension.

TRUST NO-ONE, NOT EVEN YOURSELF.

That's the motto of the security conscious developer.

Prev	Up	Next
Chapter 5. General advice and code magic	Home	DON'T: Using superglobals ($_REQUEST, $_SERVER, $_GET, $_POST etc)

Work In Progress

Write code which doesn't suck

DON'T: Hard-coded database queries

Login

Tags