Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions library/HTMLPurifier/AttrTransform/SafeParam.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,27 @@ public function transform($attr, $config, $context)
{
// If we add support for other objects, we'll need to alter the
// transforms.
switch ($attr['name']) {
switch (strtolower($attr['name'])) {
// application/x-shockwave-flash
// Keep this synchronized with Injector/SafeObject.php
case 'allowScriptAccess':
case 'allowscriptaccess':
$attr['name'] = 'allowScriptAccess';
$attr['value'] = 'never';
break;
case 'allowNetworking':
case 'allownetworking':
$attr['name'] = 'allowNetworking';
$attr['value'] = 'internal';
break;
case 'allowFullScreen':
case 'allowfullscreen':
$attr['name'] = 'allowFullScreen';
if ($config->get('HTML.FlashAllowFullScreen')) {
$attr['value'] = ($attr['value'] == 'true') ? 'true' : 'false';
} else {
$attr['value'] = 'false';
}
break;
case 'wmode':
$attr['name'] = 'wmode';
$attr['value'] = $this->wmode->validate($attr['value'], $config, $context);
break;
case 'movie':
Expand All @@ -70,12 +74,13 @@ public function transform($attr, $config, $context)
$attr['value'] = $this->uri->validate($attr['value'], $config, $context);
break;
case 'flashvars':
$attr['name'] = "flashvars";
// we're going to allow arbitrary inputs to the SWF, on
// the reasoning that it could only hack the SWF, not us.
break;
// add other cases to support other param name/value pairs
default:
$attr['name'] = $attr['value'] = null;
$attr['name'] = $attr['value'] = '';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using null values instead of strings is deprecated in functions like htmlspecialchars

}
return $attr;
}
Expand Down
28 changes: 22 additions & 6 deletions library/HTMLPurifier/Injector/SafeObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ class HTMLPurifier_Injector_SafeObject extends HTMLPurifier_Injector
'allowNetworking' => 'internal',
);

/**
* Lower-cased map for $addParam
* @type array
*/
protected $addParamMap = array();

/**
* These are all lower-case keys.
* @type array
Expand All @@ -47,6 +53,13 @@ class HTMLPurifier_Injector_SafeObject extends HTMLPurifier_Injector
'allowfullscreen' => true, // if omitted, assume to be 'false'
);

public function __construct()
{
foreach (array_keys($this->addParam) as $name) {
$this->addParamMap[strtolower($name)] = $name;
}
}

/**
* @param HTMLPurifier_Config $config
* @param HTMLPurifier_Context $context
Expand All @@ -64,11 +77,13 @@ public function handleElement(&$token)
{
if ($token->name == 'object') {
$this->objectStack[] = $token;
$this->paramStack[] = array();
$paramStack = array();
$new = array($token);
foreach ($this->addParam as $name => $value) {
$new[] = new HTMLPurifier_Token_Empty('param', array('name' => $name, 'value' => $value));
$paramStack[strtolower($name)] = true;
}
$this->paramStack[] = $paramStack;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From line 67: we've already added these parameters, so we must initialize the parameters stack with them to avoid duplicates later.

$token = $new;
} elseif ($token->name == 'param') {
$nest = count($this->currentNesting) - 1;
Expand All @@ -78,23 +93,24 @@ public function handleElement(&$token)
$token = false;
return;
}
$n = $token->attr['name'];
$n = strtolower($token->attr['name']);
// We need this fix because YouTube doesn't supply a data
// attribute, which we need if a type is specified. This is
// *very* Flash specific.
if (!isset($this->objectStack[$i]->attr['data']) &&
($token->attr['name'] == 'movie' || $token->attr['name'] == 'src')
($n == 'movie' || $n == 'src')
) {
$this->objectStack[$i]->attr['data'] = $token->attr['value'];
}
/** @TODO: fix comment */
// Check if the parameter is the correct value but has not
// already been added
if (!isset($this->paramStack[$i][$n]) &&
isset($this->addParam[$n]) &&
$token->attr['name'] === $this->addParam[$n]) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that there is a bug: the condition $token->attr['name'] === $this->addParam[$n] always evaluates to false. This has been corrected to use $token->attr['value'] instead.

isset($this->addParamMap[$n]) &&
$token->attr['value'] === $this->addParam[$this->addParamMap[$n]]) {
// keep token, and add to param stack
$this->paramStack[$i][$n] = true;
} elseif (isset($this->allowedParam[strtolower($n)])) {
} elseif (isset($this->allowedParam[$n])) {
// keep token, don't do anything to it
// (could possibly check for duplicates here)
// Note: In principle, parameters should be case sensitive.
Expand Down
16 changes: 16 additions & 0 deletions tests/HTMLPurifier/HTMLModule/SafeObjectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,22 @@ public function testFullScreen()
);
}

public function testParamsNamesNormalization()
{
$this->assertResult(
'<b><object width="425" height="344" type="application/x-shockwave-flash" data="Foobar">
<param name="allowscriptaccess" value="dummyValue" />
<param name="flashVars" value="foobarbaz=bally" />
<param name="allowfullscreen" value="true" />
</object></b>',
'<b><object width="425" height="344" type="application/x-shockwave-flash" data="Foobar"><param name="allowScriptAccess" value="never" /><param name="allowNetworking" value="internal" />

<param name="flashvars" value="foobarbaz=bally" />
<param name="allowFullScreen" value="false" />
</object></b>'
);
}

}

// vim: et sw=4 sts=4
Loading