[Home]WikiPatches/XssFixTalk

UseModWiki | WikiPatches | RecentChanges | Preferences

See /XssFix for the patch that is discussed below.

See also:

ChristophBerg? (cb@df7cb.de)


Exploit problem and working exploit:

The exploit as supplied by securityfocus didn't work on my wiki. Changing the entities in the first script tag to uri escapes (%3Cscript%3E) did the trick.


Quoting after %s:

IMHO it would be better to do the quoting in Ts() and Tss() after the replacement of %s. Do I miss something? --MarkusLude

The %s text is the "dangerous" user input. I would like to allow HTML in the script and translation file text. --CliffordAdams


Missing no-quote case:

I'm searching the script now for all Ts and Tss uses. So far I've found one that quoted when it shouldn't.:

Line 685:

  if ($idOnly && $showHTML) {
    print '<b>(' . Ts('for %s only', &ScriptLink($idOnly, $idOnly))
          . ')</b><br>';
  }

I didn't find any more such cases on my first pass through the code. --CliffordAdams

Somehow I missed this in 1.0.1, but fixed it in 1.0.3 --MarkusLude


Using QuoteHtml function:

I would prefer to use the QuoteHtml function rather than duplicate existing code. This function is intended to make user input safe, while allowing safe character entities to be used. Are there any objections to using this function? Note that QuoteHtml will not expand any already-quoted HTML, so it is safe to use multiple times on the same text. --CliffordAdams


Using QuoteHtml in the ReportError function:

As a temporary fix (on usemod.com/cgi-bin/wiki.pl and mb.pl) I've made the ReportError function use QuoteHtml on its input. I think this is a useful addition since other lazy wiki-programmers might not use the translation function for all error messages.

To make this change, simply change line 2602 from:

  print $q->header, "<H2>", $errmsg, "</H2>", $q->end_html;

to:

  print $q->header, "<H2>", &QuoteHtml($errmsg), "</H2>", $q->end_html;

I like this fix because the only code it changes is the error-reporting code (which is ordinarily never used, unlike the Ts and Tss functions). Any comments? --CliffordAdams


TsNQ idea (coding style):

I would rather duplicate the Ts function to TsNQ for the few no-quote cases rather than add a flag argument to the existing Ts function. (Personally, I strongly dislike optional arguments to functions.) In the future I might add a TssNQ function to be orthagonal. --CliffordAdams


QuoteHtml and TsNQ are fine with me. The additional $noquote argument was a hack since it relies on Perl defaulting non-existent parameters to undef. The TssNQ function will be necessary since GetDiffHTML passes the revision parameters unchecked to it. (Try diff'ing revision "12some_arbitrary_text34".)

Example exploit: http://www.usemod.com/cgi-bin/wiki.pl?action=browse&diff=1&id=WikiPatches%2FXssFix&revision=6&diffrevision=5%3Cscript%3Ealert('XSSvulnerabilityexists')%3C/script%3E --ChristophBerg?

Here you don't need the TssNQ? but the Tss then, if quoting is required. --MarkusLude


UseModWiki | WikiPatches | RecentChanges | Preferences
Edit text of this page | View other revisions | Search MetaWiki
Last edited October 10, 2007 1:52 pm by MarkusLude (diff)
Search: