[wp-hackers] Making WP more secure the evolutionary way

[Florian Thiel]

Hello everybody,

My name is Florian and I'm doing research on open source projects and
security for my diploma thesis. I would like to propose (patch
included) a very lightweight, evolutionary approach to fix or at least
mitigate SQL Injection weaknesses, some of which plagued WordPress in
the past.

Previous attempts to "just make WordPress use data access abstraction"
(as sometimes proposed on the mailing list) were understandably met
with a lot of opposition as large structural changes don't come easy
in an open source project (or any project).

I noticed that there already is basic data access abstraction in the
code (e.g. $wpdb->insert and $wpdb->update in wp-db.php) and also an
issue in the tracker that says developers should use these
abstractions more often. That's why I think you are fundamentally in
favor of these abstractions and I'd like to make the transition easier
for everybody.

I produced a patch against WordPress 2.7 which annotates and
classifies all uses of raw inline SQL. The classification tells you
how much work it would be to get rid of the inline use of SQL. The
patch can be found at
http://www.noroute.de/downloads/research/wordpress-2.7_sqlannotations.diff

>From the 443 places where I found inline SQL, there are 85 places
where an abstraction already exists and just had to be used (these are
mostly the cases which were mentioned in the issue
tracker). Furthermore, there are 172 places where a trivial
implementation (trivial meaning that a very similar method already
exists) would help get rid of the use of inline SQL. So by adding
around 5 simple methods to wp-db.php you would get rid of more than
250 problematic uses of inline SQL. And the best part: We can start
with the low-hanging fruit and gradually move to the harder ones while
keeping the code working all the time!

The process is really simple: Find a spot you'd like to fix, get rid
of the use of raw SQL (for annotations containing
"trivial_implementation", "simple_code" and "algorithmic" you may need
to write the abstraction methods first, see descriptions at the bottom
of the posting) and remove the annotation. A simple search
gives you the number of annotations remaining, so you know how for
along you are. (There's a tiny script at
http://www.noroute.de/downloads/research/sqlannotation_stats.sh that
gives you annotation count for the different classes (run it in the
root folder of the source code); works on unix only, sorry).

I'd definitely like to get feedback from you, even (or especially) if
you don't think my approach is worth it. If you have any concerns,
questions or further suggestions I'll be delighted to help. I'll be in
the loop. If this works for you I'll do another annotation set for
HTML escaping against XSS.

You can find a detailed explanation of the classes of inline SQL use
at the bottom of this posting.

Hope to hear from you,
Florian

----
Detailed description of the SQL annotation classifications:

method_exists:
This can be easily fixed by looking up the correct abstraction in
wp-db.php and applying it.

trivial_implementation:
The SQL statement does not use any "advanced" features and a similar
abstraction already exists for another SQL clause. You can look
at the existing abstractions in wp-db.php and create a new one for the
needed clause. Applies to DELETE, DROP and SELECT, etc.

simple_code:
After you have all the abstractions for trivial_implementation you can
go on implementing advanced SQL features like LIMIT, GROUP BY etc.

algorithmic:
These are the most advanced abstractions. Here you need an algorithm
that can generate complex clauses. The most prominent clauses here are
WHERE (including inequality, AND, OR, NOT, IN, parentheses and LIKE),
SELECT (with AS and functions) and JOIN.