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

[Greg Knaddison - GVS]

On Jan 23, 2008 12:33 PM, Ivan Sergio Borgonovo <mail at webthatworks.it> wrote:
> On Wed, 23 Jan 2008 11:47:34 +0000
> "Stephane Corlosquet" <scorlosquet at gmail.com> wrote:
>
> > As some pointed out, the one who reports a security issue and the
> > module maintainer(s) should be more involved in the fixing process:
> > 1- better communication and transparency between reporters,
> > maintainers and sec team
>
> This sentence got my attention.
>
> Recently I had to deal with the sec team and the experience was not
> that satisfying.

A simple +1 to Stephane's comment wold have sufficed.
He-said/she-said is rarely beneficial to either party.

The message is clear: the security team needs to scale better and have
better communication with the issue reporters and module owners.  It
would be nice if you could respond with some feedback about
Webchick/Dww's proposed process.  They've laid out a process to help
get improve the communcation and the security team is taking steps to
evaluate, improve, and implement it.

> As a side note I think that due to the success of Drupal and its
> larger community of developers this policy is not anymore suitable
> for the project:
> The Drupal philosophy - Escape or filter when appropriate
> http://drupal.org/node/101495
> on a larger and larger code base, with many newcomers "when
> appropriate" is a slimy concept and relying on db_query didn't show
> to be a "catch all" solution (not to mention XSS and other kind of
> niceness.
> Drupal API is its most valuable asset. Encapsulation makes an API
> more valuable. If people have to discover "when it is appropriate"
> the API becomes less valuable. If people can't understand when it is
> appropriate, it is risky [2].

I'm not sure I understand this criticism.  The problem you reported
was specific to contrib modules which passed in raw data even though
the API docs say to pass in an array of integers.  Heine made the 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?

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
the DatabaseAPI, the Localization system, or FAPI you also also happen
to have SQL Injection, XSS, and CSRF protection for free.  It's quite
genius in my opinion and I have a hard time of thinking how to make it
better.  We still have two major areas of weakness, IMO.  First is in
displaying data from and node/user/comment and in theming where
confusion about $node->field_something[0][value] vs.
$node->field_something[0][view] can lead to unintended security holes.
 The second major area of weakness, is in education about the
existence of the APIs and the _proper_ way to use them.

Thanks,
Greg

-- 
Greg Knaddison
Denver, CO | http://knaddison.com
World Spanish Tour | http://wanderlusting.org/user/greg