XSS on node edit form
bjaspan - June 10, 2008 - 02:07
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | node system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | bjaspan |
| Status: | closed |
Description
This bug was fixed by a previous D6 SA and never exited in D5, but has not yet been fixed in HEAD. To reproduce:
1. Create a node whose title is "alert('xss')" (use actual < and > chars, not the HTML entities).
2. Save the node.
3. Click Edit. Observe the JS popup.
The culprit:
<?php
function node_page_edit($node) {
drupal_set_title($node->title);
return drupal_get_form($node->type .'_node_form', $node);
}
?>Contrast with D6's, which calls check_plain($node->title).
Patch attached. We need a test case for this; I haven't written one yet.
| Attachment | Size |
|---|---|
| node-title-xss.patch | 2.94 KB |
#1
Delete modules/node/node.install diff ;-)
#2
can we mark as duplicate to: http://drupal.org/node/242873 ?
#3
Yes. ;) #242873: make drupal_set_title() use check_plain() by default. is the better solution to this problem.
#4
#1: Ooops, sorry about those node.install changes. That's a glimpse into the automated XSS detection approach I was toying with that revealed this bug.
#2: We can mark this a duplicate of #242873 if and only if that issue gets changed from "feature request, normal priority" to "bug report, critical priority" because this issue is an XSS security vulnerability. It is simpler just to fix this bug now by adding in the check_plain(), matching the current API, and then removing it in the patch for #242873 if/when that patch gets committed.
#5
#6
Same patch with a test case added.
#7
flobruit: Thank you!
#8
Still applies.
#9
Need to be re-rolled.
#10
Committed to CVS HEAD. Thanks all, and sorry for the delay.
#11
Automatically closed -- issue fixed for two weeks with no activity.