I first filed this issue in the private security queue but was given the go ahead to report this in the public issue queue as there is no official release yet for the 7.x branch.

The 7.x-2.x branch of the Active Tags module has an XSS vulnerability. The $term->name are output as is, without being wrapped with check_plain():

function theme_active_tags_term_list_remove($variables) {
    ...
    $output .= '<div id="at-term-' . $term->tid . '" class="at-term at-term-remove"><span class="at-term-text">' . $term->name . '</span><span class="at-term-action-remove">x</span></div> ';
    ...
}

function theme_active_tags_term_list_add($variables) {
    ...
    $output .= '<div id="at-term-' . $term->tid . '" class="at-term at-term-add"><span class="at-term-text">' . $term->name . '</span><span class="at-term-action-add">x</span></div> ';
    ...
}

There is some rudimentary sanitation done in javascript but this is not sufficient:

activeTags.addTerm = function (context, term) {
  ...
  // Removing all HTML tags. Need to wrap in tags for text() to work correctly.
  term = $('<div>' + term + '</div>').text();
  term = Drupal.checkPlain(term);
  term = jQuery.trim(term);
  ...
};
  • It is possible to add tags without adding them via javascript, by entering the data and then saving the form without first clicking on "Add tag".
  • The tags can be edited outside the scope of this widget, eg by editing them in the Taxonomy module.

The danger is mitigated since this can only target people that have permissions to edit taxonomy terms through the active tag widget.

Comments

pfrenssen’s picture

Status: Active » Needs review
StatusFileSize
new4.95 KB
greggles’s picture

The last commits to this module were 2 years ago - its possible that it is abandoned.

Would you consider becoming a co-maintainer? If so, check out https://drupal.org/node/251466

pfrenssen’s picture

Sure, I have created an application in #2288987: Offer to maintain Active Tags. I will also reach out to dragonwize and darrenmothersele by email.

sylvainm’s picture

Patch of comment #1 is not a patch file, here is a new patch :)

pfrenssen’s picture

Thanks! I don't know what got into me to upload some random garbage and pretend it to be a patch :)

pfrenssen’s picture

Status: Needs review » Fixed

  • pfrenssen committed bacfb47 on 7.x-2.x authored by SylvainM
    Issue #2288251 by pfrenssen, SylvainM: Fix XSS vulnerability when...
pfrenssen’s picture

Created an initial 7.x-2.0-alpha1 release that contains this security fix. Since this was only affecting the development branch no official security release will be made and a CVE will not be issued. Still, all users of the current 7.x-2.x branch are advised to update to 7.x-2.0-alpha1 as soon as possible.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.