[drupal-devel] [bug] XSS vulnerability in comments

[Morbus Iff]

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

 Project:      Drupal
 Version:      cvs
 Component:    comment.module
 Category:     bug reports
 Priority:     critical
 Assigned to:  Mogurito
 Reported by:  Mogurito
 Updated by:   Morbus Iff
 Status:       patch (code needs review)

Mogurito: what are your input filters configured as, and what version of
Drupal are you running? My two concerns are: you don't appear to be
running the default Drupal comment input filter (which restricts
allowed tags to A B I EM STRONG UL OL LI - see also my H1 test on that
comment). Can you duplicate this behavior on a new install of Drupal
4.6.3? I am unable to duplicate this using today's HEAD, with the
default "Filtered HTML" input format (in fact, I get "Terminated
request because of suspicious input data." If I switch this over to the
"Full HTML" input format, which supports, uh, Full HTML (and every
nastiness that entails), then it DOES work.




Morbus Iff



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

Wed, 14 Sep 2005 12:12:05 +0000 : Mogurito

Attachment: http://drupal.org/files/issues/comment.module_13.diff (632 bytes)

The source code interpreted by the browser, have html comment with the
subject by each comment. For example, if we post a comment like
'Hello', Drupal write in the source code . But the problem come when a
post is send with special characters like 'Hello XSS', the comment in
the source code will be closed, provocating that "Hello XSS-->" string
will be show in the page. If the "Hello XSS" string will be replaced by
a script, we can get a XSS Attack.


*Solution*
Analize the subject string and replacing the special characters (, ')
for (<, >, ")


*How*
Adding this lines to the function comment_validate_form:
$edit['subject'] = ereg_replace("", ">", $edit['subject']);
$edit['subject'] = ereg_replace("\'", """, $edit['subject']);


*Notes*
- Sorry, but my english is not very cool.




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

Wed, 14 Sep 2005 12:22:34 +0000 : Mogurito

*CORRECTION* (The comment is not complete)


The source code interpreted by the browser, have html comment with the
subject by each comment. For example, if we post a comment like
'Hello', Drupal write in the source code . But the problem come when a
post is send with special characters like '<'. If we post a comment
like '-->Hello XSS', the comment in the source code will be closed,
provocating that "Hello XSS-->" string will be show in the page. If the
"Hello XSS" string will be replaced by a script, we can get a XSS
Attack.


Solution
Analize the subject string and replacing the special characters (<,>,
') for (&<, &>, &")


How
Adding this lines to the function comment_validate_form:
$edit['subject'] = ereg_replace("<", "&<", $edit['subject']);
$edit['subject'] = ereg_replace(">", "&>", $edit['subject']);
$edit['subject'] = ereg_replace(""", "&"", $edit['subject']);


Notes
- Sorry, but my english is not very cool.




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

Wed, 14 Sep 2005 13:13:38 +0000 : Thox

Attachment: http://drupal.org/files/issues/comment.module_27.patch (1.04 KB)

I imagine it is wiser to handle this with check_plain() in
theme_comment(), the same as it is handled in theme_node(). See
attached patch against CVS HEAD.




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

Wed, 14 Sep 2005 13:17:30 +0000 : tostinni

Can you give us a proof of concept, because, I can't reproduce it.
If I put something between a <script> tag, it is stripped out.


In fact everything rely on the action of filter module, which take care
of stripping undesirable code. And I think that it's doing a good job.




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

Wed, 14 Sep 2005 13:57:35 +0000 : Mogurito

Yes, see this link [1]. I posted a comment with a subject that containt
a script. See the comment.
[1] http://www.agali.org/XSS_BUG




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

Wed, 14 Sep 2005 14:03:09 +0000 : eldarin

Quite serious implications ...
This would of course allow someone to pick up all the user passwords
being typed on pages !!!
The worst one of these is of course the admin user and password. It's
very easy to get some DOM-handling and attaching to the submit of the
user login form.


8^O