[development] Think there's a security problem in your module? Here's what to do.

Ivan Sergio Borgonovo mail at webthatworks.it
Thu Jan 24 02:27:01 UTC 2008

On Wed, 23 Jan 2008 15:04:56 -0200
"Greg Knaddison - GVS" <Greg at GrowingVentureSolutions.com> wrote:

> call to fix this inside the API so that it was liberal in accepting
> any data and would do the filtering to prevent the holes as much as
> possible.  That sounds like it does what you want.

> Can you state in a positive manner what change you would like to
> see? Do you feel that prepared statements will be the solution to
> this problem?

It is just a db_query on steroids.
If you still compose the prepared statement dynamically... you still
get the chance to inject code but you don't cast in php, you should
be able to use NULL (great once you start to use pk/fk) and you make
the code of db_query much simpler making it easier to make it smarter.

Improving the abstraction of the DB layer should avoid most chances
to get into trouble since queries will be built with prepackaged
static parts and parameters, not anymore assembling strings in the

> As much as possible the Drupal project has used so far is to have
> security as a side effect of proper use of the API: i.e. if you use


> holes. The second major area of weakness, is in education about the
> existence of the APIs and the _proper_ way to use them.

People make errors and if they have to take care about what is the
proper way to use the API they will make more errors.
If it is clearer what they can rely on it makes stuff easier and
simpler, if the things they can rely on are more than the one with a
red button with "self destruction" written on it would be even better.

function A accept int may mean:
a) function a will work just with int, explode in errors otherwise
b) function a will break silently with anything but int
c) if you let rubbish get in you may be exposed to a SQL injection

I think a) is better than b) and may be a way to avoid c) earlier; at
least things will reach the log and you've a chance to fix stuff.
Or maybe it is just a matter of making the input type parameters in
the docs <blink>.

As for a positive contribution to the sec process and for the people
asking if anything was really screwed I'd suggest:

- kill the security at drupal.org email if you can't guarantee it is a
reliable channel (spam/whatever)
- open a web form (I know there is one) and make it send an autoreply
with instructions and greetings immediately. The point is: offer just
things you know will work. Reply promptly with no effort. Lower the
chances people don't know what to do next.
- just offer *official* reply through official channels, so that
personality don't kick in.
- be aware that once you decide to do voluntary job and be part of
a community you take the responsibilities that come with *your
choice*. Being part of the sec team of a successful project as Drupal
is like volunteering as a paramedic.
Projects do get judged by their security response too, it is also a
matter of image. Image is important for getting more dev involved.
- keep the reporter informed [1]
- Establish a code of conduit for sec team communications: You're
dealing with someone you don't know that is aware of a security
problem. You don't know his personality, you don't know his skills,
you want information and hopefully collaboration: guide and pamper
the reporter. The consequences of an unexpected behaviour may have a
high cost in terms of sec and image.
A code of conduit may make dealing politely with the issue nearly
automatic, reducing the chances of frictions and the time required to
formulate answers and make easier to delegate responsibility to reply
if the one in charge is busy and the one left don't have the gift of

Thanks to Derek who gave me just one more kick in my ass and woke
me up from my dream, giving me one more opportunity to test if
finding the positive side of all negative experiences is a rewarding
hobby and unconsciously reminding me that computers are used by
people and taking care of the human factor in security is a big slice
of the equation and it is worth to insist on it.

[1] this should address also the problems related to how to
manage disclosures and how to fix the issues in a more elastic way
even if you still may have a "preferred" or "suggested" way to deal
with security

Ivan Sergio Borgonovo

More information about the development mailing list