[uf-dev] MediaWiki extension to support classes in anchor tags

[Jim Wilson]

Hi Jeff,

Please excuse the length of this email.  I think it's really great
that you're doing MediaWiki extension development, and I encourage you
to keep it up!

I noticed another of your extensions at mediawiki.org having to do
with "enabling" <abbr> tags.  Unfortunately, MediaWiki does not make
it easy to simply "enable" these kinds of things - causing a vacuum to
be filled by extensions such as yours.  It has been on my TODO list
for a while to modify the Parser and Sanitizer classes to give wiki
administrators granular control over which tags/attributes are
allowed, but alas I haven't gotten around to it :/

In any case, here are my thoughts on your extension as it is today:

Coding Observations:

* The following line should be removed from the Extension code (it is
unnecessary):

  $wgHooks['ParserAfterStrip'][] = 'extendAnchorTag';

* The comments mention matching the href attribute against
$wgUrlProtocols, but that variable isn't actually checked.  Instead, I
suggest validating against the result of wfUrlProtocols() since this
will handle the concatenation for you, and allow the wiki admin to
retain control over the allowed protocol list.

* There's a lot of code duplication in the tag rendering portion of
startExtendAnchor().  This can be shortened greatly (code sample
provided at the end).

* Also, it's legal in MediaWiki to create extension tags with the
names of real actual tags, so feel free to use <a> instead of <xa> if
you feel it's more appropriate - it's totally up to you.

Security Observations:

* I notice you use htmlspecialchars() to sanitize user input prior to
display.  This is a great first step.  To be even more effective, you
probably want to set the optional second parameter to ENT_QUOTES.
This will ensure that single quotes will be encoded along with double
quotes.  For non-url parameters (like target or class), I usually
recommend wfUrlencode(), which builds on PHP's native urlencode()
method.  Also note that htmlspecialchars() is often not sufficient to
stop XSS attacks (as illustrated by the following example).

* If the internal anchor text starts with "<img ", it is kept after an
htmlspecialchars() cleansing. I believe this indicates that you mean
to allow images.  Unfortunately this permits the user to hotlink
external images (which a wiki administrator may want to prevent).
Secondly, this provides a convenient injection vector for arbitrary
JavaScript.  To see why, consider this markup:

  <xa href="#"><img src='http://jimbojw.com/images/transparent.png'
onLoad='alert("XSS Code Here...")' /></xa>

When added to a wiki page, this will execute the alert() after the
image has loaded, meaning after it has downloaded from the server (or
been retrieved from the browser cache).

The alternative I'd suggest is using the Parser's recursiveTagParse()
method to evaluate the $input.

So, without further ado, here is my proposed implementation of
startExtendAnchor():

----------------------------------------
function startExtendAnchor( $input, $argv, &$parser ) {

    # Short-circuit if required 'href' param is missing
    if (!isset($argv['href']))
        return "<div class='errorbox'>Error: <tt>href</tt> attribute
missing for <tt>&lt;xa&gt;</tt> tag.</div>";

    # Short-circuit if a bad protocol is encountered
    if (!preg_match( '/^(#|'.wfUrlProtocols().')/', $argv['href']))
        return "<div class='errorbox'>Error: Bad protocol specified in
<tt>href</tt> attribute of <tt>&lt;xa&gt;</tt> tag.</div>";

    # Set aside $href and sanitize the rest of the $argv array
    $href = htmlspecialchars( $argv['href'], ENT_QUOTES);
    $argv = array_intersect_key( $argv, array( 'target'=>1,
'class'=>1, 'rel'=>1 ) );
    array_walk( $argv, 'wfUrlencode' );

    # Build and return anchor markup
    $anchor = "<a href=\"$href\"";
    foreach ( $argv as $attrib => $val ) $anchor .= " $attrib=\"$val\"";
    return $anchor . " >" . $parser->recursiveTagParse($input) . "</a>";

}
----------------------------------------

I'll be happy to answer any questions, and good luck!

-- Jim R. Wilson (jimbojw)

On 10/20/07, Jeff McNeill <jeff at jeffmcneill.com> wrote:
> Hi folks,
>
> I hacked a MediaWiki extension for class support in anchor tags...
> http://www.mediawiki.org/wiki/Extension:ExtendAnchorTags
>
> Any and all input or suggestions welcome.
>
> --
> Sincerely,
> Jeff McNeill
> http://jeffmcneill.com/
> _______________________________________________
> microformats-dev mailing list
> microformats-dev at microformats.org
> http://microformats.org/mailman/listinfo/microformats-dev
>