[wp-hackers] Making WP more secure the evolutionary way
wordpress at santosj.name
Thu Jan 22 14:28:46 GMT 2009
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
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
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.
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
>> 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?
More information about the wp-hackers