XSS on node edit form

Posted by bjaspan on June 10, 2008 at 2:07am

Issue Summary

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	Status	Test result	Operations
node-title-xss.patch	2.94 KB	Ignored: Check issue status.	None	None

Comments

#1

Posted by lilou on June 10, 2008 at 2:14am

Delete modules/node/node.install diff ;-)

Attachment	Size	Status	Test result	Operations
node-title-xss_1.patch	656 bytes	Ignored: Check issue status.	None	None

#2

Posted by pwolanin on June 10, 2008 at 2:15am

can we mark as duplicate to: http://drupal.org/node/242873 ?

#3

Posted by dww on June 10, 2008 at 6:29am

Status:

needs review

» closed (duplicate)

Yes. ;) #242873: make drupal_set_title() use check_plain() by default. is the better solution to this problem.

#4

Posted by bjaspan on June 10, 2008 at 1:16pm

Status:

closed (duplicate)

» needs review

#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.