Drupal Warns Users of Mass, Automated Attacks On Critical Flaw

Trailrunner7 writes The maintainers of the Drupal content management system are warning users that any site owners who haven't patched a critical vulnerability in Drupal Core disclosed earlier this month should consider their sites to be compromised. The vulnerability, which became public on Oct. 15, is a SQL injection flaw in a Drupal module that's designed specifically to help prevent SQL injection attacks. Shortly after the disclosure of the vulnerability, attackers began exploiting it using automated attacks. One of the factors that makes this vulnerability so problematic is that it allows an attacker to compromise a target site without needing an account and there may be no trace of the attack afterward.

Comments

Actual irony?

[TWX]

SQL injection flaw in a Drupal module that's designed specifically to help prevent SQL injection attacks

Would this be actual irony, as opposed to Alanis Morrissette irony?

Re:Actual irony?

[bill_mcgonigle]

Would this be actual irony, as opposed to Alanis Morrissette irony?

That a song with that name contains no actual examples of irony is ______.

This message brought to you by Deep Metathinking and the Number 12.

Re:Actual irony?

[unrtst]

... roll-your-own implementations are likely to be broken too.

As far as I can tell, this module uses custom placeholders in queries, and then replaces those with the user supplied values, building a string that can be passed to the DB as SQL without database placeholders. IE. it's not building something like:

        $db->prepare("SELECT name FROM table WHERE something IN (?,?,?)")
        $db->execute( @parameters );

It's building something like:

        $db->prepare("SELECT name FROM table WHERE something IN ($param[0], $param[1], $param[2])")

That's always more risky. DB placeholders are not a silver bullet, but they're damn close. /disclaimer, I didn't thoroughly audit the code, so maybe it is somehow using db placeholders, but the method in question doesn't look like it is.
See line 739 here: http://cgit.drupalcode.org/dru...
Patch for users that don't want to do a full upgrade and are on 7.0 - 7.31: https://www.drupal.org/files/i...

Re:Actual irony?

[carrier lost]

Rain on a wedding day isn't ironic...

Agreed.

Irony is rain on the wedding day of a couple of meterologists.

I may be wrong, but I think that coincidence is 2-factor, irony is 3-factor

What about Drupal 6?

[joelsherrill]

The story only mentions Drupal 7. Is Drupal 6 or 8 impacted?

Re:What about Drupal 6?

[yelvington]

Drupal 6 does not use the affected abstraction layer.

Re:What about Drupal 6?

[MightyMartian]

Gods save us from poorly-designed abstraction layers designed to do things "better".

Re:What about Drupal 6?

[meustrus]

Does not affect Drupal 6. Not sure about Drupal 8, but if you're concerned about the security of your website run on beta releases you're doing it wrong.

Re:PHP

[benjymouse]

How do prepared statements handle the not uncommon situation where you want to include an "in" clause? For example:

select * from customers where city in ?citylist

This was the problem they tried to solve by dynamically creating a statement like:

select * from customers where city in (?city-1, ?city-2, ?city-3)

So, to generate the -1, -2, and -3 parts they relied upon the index of the array.

Only in PHP an array will turn around and bite you with it's dual personality as a hash table. A hash table where one key was not "-1" but rathersomething like (pseudo):

-1); drop table students; --

You cannot really fault the Drupal developers for trying to support this commonly occurring pattern, for which there are no good solutions with plain prepared statements. After all, if they could write secure code for a common problem that could prevent less experienced developers for falling back to error-prone and insecure string interpolation.

Don't get me wrong: The drupal developers is at fault. But they were set up by the criminally insecure PHP.

Re:PHP

[Jaime2]

Microsoft SQL Server has both an XML data type and a table-valued parameter that can be used to pass an arbitrarily long list of values in a single parameter. Does MySQL not have an equivalent, or maybe it does and PHP doesn't support them?

At this surprises who?

[Mysticalfruit]

I'm surprised it took this long! While not a PHP programmer, I've looked at some bits of the code and it's a bloody mess.

php should get a new motto: "Please Hijack our Platform"

Re:HAHAHA Little bobby tables

[meustrus]

Tip to moderators: There is no mod category "Sad". The best response is to ignore it, because then those who can recognize a completely unmoderated post will appreciate the metahumor.

Valuable lesson learned

[NaughtyNimitz]

I did some websites in Drupal, but now I am steering clear of Drupal and the likes (Wordpress,...)

Now 100% of my projects are in my custom CMS where obfuscation is the rule.

Re:Valuable lesson learned

[drinkypoo]

Now 100% of my projects are in my custom CMS where obfuscation is the rule.

So now instead of many eyes on your CMS, there are only yours? People who keep up with their updates don't really have to worry about this. I used to check my site status page daily, but I noticed that I get notified of all the major Drupal patches by Slashdot, which is handy.

Re:PHP

[unrtst]

How do prepared statements handle the not uncommon situation where you want to include an "in" clause? For example:

select * from customers where city in ?citylist

This was the problem they tried to solve by dynamically creating a statement like:

select * from customers where city in (?city-1, ?city-2, ?city-3)

So, to generate the -1, -2, and -3 parts they relied upon the index of the array.

...

for which there are no good solutions with plain prepared statements.

...

Bullshit. Psuedo code cause I'm too lazy to look up the php-ism for this:
$stmt = "select * from customers where city in (".join(',', map { '?' } array_values($city_list) ).")";
$sth = $db->prepare($stmt);
$sth->ececute(array_values($city_list));

Wrapper code to aid in building the placeholder stuff should be used to account for max count of items (generally 255 of them), after which it should split it to:
( city in (?,?,?... etc ...) OR city in (?,?,? ... etc ...) )

Does that take work? yes. Is it more effort than what they're doing? no.

WhiteHouse.gov

[q4Fry]

Is the White House breach a result of this bug? Inquiring minds want to know!

Re:PHP

[Qzukk]

XML would not be a standard SQL construct. Neither the PHP-internal mssql driver nor the microsoft PHP driver supports TVP.

The postgresql way to prepare a statement that needs to do something like "... field IN ($1) ..." is to rewrite it as an array operation "... field = ANY ( $1 ) ..." where $1 would be an array, but PHP/PDO can't properly/securely prepare this since it doesn't understand array operations. You would need to manually escape each element and create a literal array string in your code and pass that as the parameter:

pg_prepare($pg, "test", "select * from customer where id = ANY ( $1::int[] )");
pg_execute($pg, "test", array("{52,149,288}"));

Note that a varchar[] in PHP would look something like "{Smith,O'Hare,Wilkerson\\, Esq.}" so none of the normal SQL escaping functions would work properly (note that single quotes are not escaped, but commas and curly braces would be escaped).

I think postgresql arrays are slightly nonstandard (you can declare them using "datatype ARRAY[size]" but postgresql does not enforce array bounds. MySQL does not do array datatypes at all.