[drupal-devel] [bug] comment preview "Required" is easily bypassed

Tue Aug 9 12:27:26 UTC 2005

Issue status update for 
http://drupal.org/node/28420
Post a follow up: 
http://drupal.org/project/comments/add/28420

 Project:      Drupal
 Version:      cvs
 Component:    comment.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  Jeremy at kerneltrap.org
 Updated by:   Jeremy at kerneltrap.org
 Status:       patch (code needs review)
 Attachment:   http://drupal.org/files/issues/comment.module_13.patch (1.33 KB)

> I think it misses something, like some timestamp related hash, because
> once you get the hash code you can post multiple comments with that.

Using a timestamp will mean that the comment form "expires".  That is,
if you wait too long to preview your comment, it will generate an error
when you try to post.

Yes, technically a spammer could post one real comment, then based on
what was in the session from that they could post the same identical
comment over and over, so long as it was attached to the same node. 
But this is not what they do, they try and spread their spam throughout
your webpage.  Furthermore, the spam module is perfectly capable of
detecting and preventing this.

> Another problem I can think of is, what happens when a cron run
happens
> between the preview and the post?? I'm afraid comments would get lost

The key is only generated once, that's what the first test is about. 
In any case, in the unlikely event that the key were to change between
preview and post they would simply have to post a second time.

My earlier patch wasn't quite right, I was testing the token in the
wrong place.  This patch fixes that.

BTW:  This is beneficial for maintaining high quality submissions too,
as prior to this change someone could:
  1) enter a comment
  2) press preview
  3) completely change their comment (introducing a mistake)
  4) press submit and the comment (mistake and all) would go into the
database unpreviewed

After this change:
  1) enter a comment
  2) press preview
  3) completely change their comment (introducing a mistake)
  4) press submit and they get an error because they didn't preview
their changes - forcing them to preview once after any change

Jeremy at kerneltrap.org

Previous comments:
------------------------------------------------------------------------

Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy at kerneltrap.org

Setting "Preview comment" to "Required" does not strictly require that
the comment be previewed first.  This is being abused by spammers to
quickly and efficiently post spam comments.

I discovered this after I added a new feature to my new spam module [1]
to auto-blacklist spammer IP addresses, allowing me to block comment
spammers when they preview a comment and thus preventing them from ever
inserting their spam into my database.  I configured my comment module
to "require" comment previews, and yet found that the comments were
slipping past my filter.  I finally realized what the spammer is doing
is setting $_POST['op'] to 'Post comment', effectively bypassing the
preview phase.

I'm currently looking for a clean solution to this.  At the moment the
only idea I have is to generate a token at the preview phase, and
validate the token at the post phase.  Unfortunately the token would
have to be stored in the databse between the preview and the post,
which adds overhead.

Alternatively, I've considered using a time-based hash which would
constantly update depending on the time of day.  This could easily be
validated without storing anything in the database.  If too long has
gone between the preview and the post, an additional preview step would
be required...  The down side here is that the time-based hash would be
publically available, and thus the spammer could easily duplicate it in
their script.  A private key could solve for that, but increases the
complexity as it adds a configuration step.

I have the feeling I'm missing a simpler, cleaner solution. 
Suggestions?
[1] http://kerneltrap.org/jeremy/drupal/spam/

------------------------------------------------------------------------

Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman

even if you get this fixed, won't these bots just add a preview step?

this 'preview required' feature is designed to maintain high quality
submissions by forcing users to proof read. it isn;t designed for
security.

i think you want to hook into comment_validate(). just add a hook here
- there is already a hook_comment() waiting for you to add an
operation.

------------------------------------------------------------------------

Mon, 08 Aug 2005 08:49:30 +0000 : Eaton

I posted a patch a few days ago (http://drupal.org/node/28255) that adds
validation and form construction hooks for comments. It's similar to the
one that the captcha module uses, though it adds comment form_pre and
form_post hooks instead of a single comment form hook.

------------------------------------------------------------------------

Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy at kerneltrap.org

Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB)

> even if you get this fixed, won't these bots just add a preview step?

Eventually, yes, but it drastically changes their ability to fling spam
at a site.  As is, they simply have a script that shoots data out at
high speed without having to wait for messages to return from the
server.  It is the server that is doing all the work, thus making it
simple for a spammer to DoS a site.

If "preview required" really meant "preview required", they would be
forced to first automate clicking "preview", and then wait for a
response before clicking "submit".  This requires more resources on
their side, and allows us to add delays after clicking "preview" (if we
detect that they are a spammer) further using their resources.

> this 'preview required' feature is designed to maintain high quality
> submissions by forcing users to proof read. it isn;t designed for
security.

Regardless of the intention, I was misled to believe that configuring
my site to require previews would require that all comments were first
previewed.  As a site administrator, I would prefer to know that
"required" really means "required".

> i think you want to hook into comment_validate(). just add a hook
here -
> there is already a hook_comment() waiting for you to add an
operation.

Yes and no.  Ultimately yes that will work and will allow my spam
module to prevent the spam from ever being posted.  But it still leaves
the greatest burden on the web server, instead of on the spammer.  The
spammer can still use a very simple script that only pushes data, and
thus can generate spam at an unbelievable rapid rate.

Here is an example patch to enforce "preview required".  It's one idea,
I'm sure there are better ones.

------------------------------------------------------------------------

Mon, 08 Aug 2005 14:01:38 +0000 : Jeremy at kerneltrap.org

Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB)

Here's a second version of the patch that doesn't require any manual
configuration.

------------------------------------------------------------------------

Mon, 08 Aug 2005 16:27:27 +0000 : Jose A Reyero

I like this idea, and the patch looks good

Still, I think it misses something, like some timestamp related hash,
because once you get the hash code you can post multiple comments with
that.

Another problem I can think of is, what happens when a cron run happens
between the preview and the post?? I'm afraid comments would get lost

For this second problem, I think a key generated only once after module
activation could do. About the first one....mmm... I'll sit down for a
while and think.....