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

Florian Thiel flo.thiel+wphackers at googlemail.com
Thu Jan 29 00:24:26 GMT 2009


On Wed, Jan 28, 2009 at 9:59 AM, Peter Westwood
<peter.westwood at ftwr.co.uk> wrote:
>
> The real issue is that the core WordPress code already has the right level
> of functionality available to write secure queries.

I'm sorry, my first approach was probably further away form the
current workings of WP (and the development) as I thought. Do we
generally agree that a *simple* abstraction layer is a good idea from
a maintainability point of view? The existing ticket about
INSERT/UPDATE (http://trac.wordpress.org/ticket/6836) suggests as
much...


I created a simplified version of my patch
(http://trac.wordpress.org/attachment/ticket/6836/wordpress_sqlannotations_simple.diff)
which marks all the remaining occurrences of the raw INSERT/UPDATE
queries ("method_exists"). This can be used by developers to find the
incriminating queries. The patch also marks the queries which can be
moved to an abstraction layer with a trivial implementation in
wp-db.php (mostly DELETE or ALTER).

The queries can be moved incrementally so there's no big-bang. The
patch is intended to ease the transition for the developers as it
visualizes the remaining work and makes the queries searchable.

> This functionality is already used heavily although there are someplaces
> where queries may still need converting.

This is what my current patch tries to accelerate. Do you think it is
useful for that?

Since DELETEs and simple SELECTs can be abstracted analogous to the
existing methods for INSERT/UPDATE I created another patch that
implements the abstraction for DELETE and replaces a raw DELETE query.
The patch is for demonstration/feedback purposes. I'd like feedback on
if you think that this is a good next step or if it increases
complexity for developers too much...
(http://trac.wordpress.org/attachment/ticket/6836/wordpress_delete_wpdb.diff).

If all that annotation stuff just sounds like a lot of academic hassle
to deal with, please tell me so. My theory is that these kinds of
annotations (not only for SQL, there are probably many more
applications) visualize spots in the code and can ease large changes
(since changes can be incremental and it is clearly visible when
you're done; also, the annotations act as a reminder and should
encourage developers to fix the issue and get rid of the annotation
quickly).

(On the other hand, if you know another change to the WP code that
could benefit from these annotations, please tell me; I will gladly
think about an annotation style that might help).

Good night for now,
Florian


More information about the wp-hackers mailing list