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

[Jacob Santos]

The Active Record Pattern already has many (3: Zend Framework, ADODB, 
and ADODB Lite, also part of other frameworks (Yii, Code Igniter, etc) ) 
libraries devoted to it. Basically, the idea of using it for security is 
pointless based on the idea of abstracting everything. Yeah sure, you 
can use something like mysql_real_escape_string() and protect the 
database from SQL injection attacks.

This will not protect against XSS attacks or CSRF attacks. Those are 
really independent of the database abstraction, you'll really just want 
to focus on protecting the database, which prepare does a lot to do in 
the first place. I suppose the reason the XSS attack was fixed in one 
place was that was the one known place where it was affected. If you 
protected every place from XSS attacks, then it would really screw with 
plugins and posts that want to post YouTube or something like Google 
Analytics.

The magic escaping method was probably KSES or another of the entities 
encoding functions in WordPress. This is how it should be.

I'd be against Active Record Pattern, because of the additional overhead 
and no real benefits from it. It isn't like WordPress is actually ever 
going to add support for other RDBMS, so it isn't like it will have full 
support for other databases. Of course, WordPress could use ADODB Lite, 
it is GPL, but you know it would even more solidify that every plugin 
and some themes would have to be GPL.

Jacob Santos


Florian Thiel wrote:
> On Thu, Jan 22, 2009 at 11:32 AM, DD32 <wordpress at dd32.id.au> wrote:
>   
>> 2009/1/22 Florian Thiel <flo.thiel+wphackers at googlemail.com>:
>>     
>>>> Also, It seems to be that you're suggesting in your patch that using
>>>> raw SQL (even though its prepared) is a bad idea?  Or am i reading it
>>>> wrong? :)
>>>>         
>>> Yes, that's my point, I'm probably not making that clear enough. The
>>> whole idea is centralising access to the database. When you have 5
>>> places in the system where the db is accessed, it's really easy to fix
>>> things once you notice there is a security vulnerability. You're
>>> fixing it in one place and all callers profit from that. Experience
>>> shows that if you have critical things like db access all over the
>>> place you will forget updating one or two places when applying a
>>> security fix. When you look at the last XSS incidents with WP, there's
>>> the exact pattern. The problem was fixed by applying another magic
>>> escaping method to the vulnerable spot. This probably leaves other
>>> places vulnerable because escaping is not done in a consistent way
>>> everywhere it's used. Obviously, you still need to find all the places
>>> where db access (or client output) happens. But you only have to do
>>> that one time.
>>>       
>> Thats the thing, I dont see how this is different from current. All
>> queries which use user-data go through the wpdb::prepare() function,
>> which sanitizes the data for the query.. wpdb->prepare("SELECT a, b, c
>> FROM table WHERE a = %s", ' '\ UNION...'); would result in the -exact-
>> variable passed in being quoted -correctly-, Of course, since
>> wpdb::query() uses addslashes() instead of mysql_real_escape_string()
>> There may be chars which make it through that.. But thats beside the
>> point, Do you see where my confusion lies?
>>     
>
> Using prepare everywhere is definitely a step in the right direction.
> The problem is that it does not help against flawed SQL passed from
> the caller. If you do it like it's done for INSERT and UPDATE now, you
> control the construction of SQL statements in one place and can avoid
> constructing evil SQL. Since you have an abstract representation of
> the statement (semantically richer) you have much more control over
> the SQL generated. You could even do fancy things like making sure
> that all columns mentioned in SELECT or WHERE really exist. We don't
> have to go that far, but once it's centralized, improving these
> central methods has a much larger payoff, since you only have to do it
> once. That justifies spending more time on this, creating a more
> secure WordPress in the end.
>
> Florian
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers
>