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

[Jacob Santos]

The problem is that you are stated XSS attacks, which are unrelated to 
database attacks. What you are trying to prevent, is SQL injection, 
which is what prepare() does. Which isn't a perfect method, however 
neither is yours. You are proposing we abstract and do it for the 
developer, like they are idiots. You are suggesting we do it for 
WordPress core, which means the core developers and contributors are 
idiots for not protecting against SQL injection (which all of the places 
I've seen already protect against SQL injection using prepare).

This ignores true hacking prevention known as sanitization. It is really 
simple to sanitize integers, dates, and what not, because they have a 
set format which can be validated very easily and sanitized. The issue 
is actually strings, such as HTML, which can exist in posts, as well as 
other string formats which need to be accepted, but still protected. 
What you have is something that needs to be sanitized liberally based on 
the user role and capability. WordPress already does this, so I guess my 
confusion is and what I should be question is: Are you proposing to 
abstract and protect the users and developers from themselves with the 
possibility of entering strings that might be venerable to XSS attacks?

The contention I have is that I come from the understanding that each 
validation and sanitization should be created for each variable. For 
example, if the database field is a date, I will validate and, or 
sanitize for date before placing it in the SQL. If I'm using integers, 
I'll cast to an integer. I come from the group that you shouldn't 
abstract all data types into the same group, when you have multiple 
groups and multiple validations and sanitizations. So, if you protect 
each string against XSS attacks, you are ignoring the date formats. This 
could be sensed by the abstraction, but that will add overhead that 
would be unacceptable.

So I mean, you are talking about protecting against XSS attacks in the 
database layer, when it belongs to the layer directly above database 
layer, which interacts with the database layer. Adding something that 
needs to be separate from the database layer to the database layer will 
add protection which doesn't exist. If you do it this way, then no one 
will have freedom in adding JavaScript to their posts and you'll need 
something to override this.

The reason sanitization is added to one area and not others, is because 
it shouldn't be in other areas, because such protections will prevent 
freedoms that would otherwise be abstracted away from the users and 
developers. I can see the bug reports now (as editors and contributors 
who file bugs that they can't enter certain HTML elements) from 
developers and users saying that they can't enter video or [enter some 
future or current popular site] snippet into their post.

The point with security is that there are already functions inside of 
WordPress which exist to protect against certain XSS and also 
protections in the administration to protect against CSRF attacks. So 
for example, if you go to a WordPress blog and try to enter some 
JavaScript that steals the cookies, it won't execute, because the 
comment functions (and not the database methods) protect against XSS 
attacks. This is the way it should be.

The problem with security is that it isn't cut and dry, unless you don't 
want to accept any user input at all.

I would also advise against using CakePHP, which holds your hand and 
empties your garbage for you, as an example in the future as "best 
practice" because I don't really buy that CakePHP is best practice. It 
isn't for performance reasons, however neither is WordPress and in fact 
a lot of the developers suggest features and implementations which would 
make WordPress even more slow, so take what you will. I'm not the most 
intelligent developer on this list, nor the best, but I do read a lot 
and I do give advice based on other intelligent developers. I'll tell 
you now, that from what I've read, this sort of generalization of 
security is NOT the way to go, because it offers a false sense of security.

Jacob Sanots


Florian Thiel wrote:
> On Thu, Jan 22, 2009 at 1:44 PM, Jacob Santos <wordpress at santosj.name> wrote:
>   
>> I apologize, I recognized some of the functions you were proposing as those
>> usually found in database abstraction (which I confused with Active Record
>> (!= ADODB) ). When I think data-access abstraction (Documentation might be
>> incorrect on php.net, PDO appears to be database access abstraction with
>> some touch of data access abstraction in that you can use the same methods
>> to access queries in any database), I think of PDO and it certainly doesn't
>> have an insert, update, where or select method. It has prepare method.
>> Actually query is actually a prepare that is abstracted from the developer.
>>     
>
> No problem.
>
>   
>> Direct SQL in and of itself is not a bad thing, because if you know what you
>> are doing and protect against hacking attempts, then there is no reason to
>> further extend it by abstracting the SQL out. You are adding overhead where
>> none is really needed. Your requirements have already been met by using
>> prepare().
>>
>> What you seem to be worried about are plugin authors, which don't know about
>> or don't use prepare() when using SQL. If they aren't, then they probably
>> aren't protecting against other attacks.
>>     
>
> No, I'm not especially worried about plugin developers. And I'm not
> especially worried about people who don't know about prepare().
> Experience shows that these things "just happen" (tm). That's where I
> was going with the XSS example. WordPress developers seem to know
> about the problem of XSS in general, but sometimes you forget to
> sanitize your output (or input in the SQL case) or use the wrong
> sanitation. If you use abstraction, the right thing has to be done in
> one place only. If everybody was perfect all the time, your argument
> would be totally valid. But we're not. That's why reducing redundancy
> (in the end, that's what these abstractions do) is a good thing. It
> makes code more readable, more maintainable and more secure.
>
> Comet Support has an article about how SQL abstraction is handled in
> CakePHP (http://cakephp.prometsupport.com/tag/sql-injection/). This is
> a good example how powerful such an abstraction can be. NOTE: I'm not
> suggesting WP should absolutely do it that way. I just want to give an
> example how powerful an abstraction can be (note that you can
> guarantee correct SQL because you're building the statement in the
> abstraction method).
>
> Still unconvinced?
> Florian
>   
>