Commit dbdad096 authored by Oliver Hader's avatar Oliver Hader
Browse files

Fixed bug #1985: XSS vulnerability in wizard classes

git-svn-id: https://svn.typo3.org/TYPO3v4/Core/branches/TYPO3_4-1@8399 709f56b5-9817-0410-a4d7-c38de5d9e867
parent 59b98eb0
......@@ -15,6 +15,7 @@
* Fixed bug #13885: XSS in indexed search BE module (thanks to Benjamin Mack)
* Fixed bug #15254: Extension Manager allows to edit arbitrary files if noEdit flag is not set (thanks to Helmut Hummel)
* Fixed bug #14389: phtml is also PHP extension and should be denied editing / uploading via fileadmin (thanks to Ernesto Baschny)
* Fixed bug #1985: XSS vulnerability in wizard classes
2010-05-17 Oliver Hader <oliver@typo3.org>
......
......@@ -970,6 +970,36 @@ class t3lib_div {
return substr(md5($input),0,$len);
}
/**
* Returns a proper HMAC on a given input string and secret TYPO3 encryption key.
*
* @param string Input string to create HMAC from
* @return string resulting (hexadecimal) HMAC currently with a length of 40 (HMAC-SHA-1)
*/
function hmac($input) {
$hashAlgorithm = 'sha1';
$hashBlocksize = 64;
$hmac = '';
if (extension_loaded('hash') && function_exists('hash_hmac') && function_exists('hash_algos') && in_array($hashAlgorithm, hash_algos())) {
$hmac = hash_hmac($hashAlgorithm, $input, $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']);
} else {
// outer padding
$opad = str_repeat(chr(0x5C), $hashBlocksize);
// innner padding
$ipad = str_repeat(chr(0x36), $hashBlocksize);
if (strlen($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']) > $hashBlocksize) {
// keys longer than blocksize are shorten
$key = str_pad(pack('H*', call_user_func($hashAlgorithm, $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'])), $hashBlocksize, chr(0x00));
} else {
// keys shorter than blocksize are zero-padded
$key = str_pad($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'], $hashBlocksize, chr(0x00));
}
$hmac = call_user_func($hashAlgorithm, ($key^$opad) . pack('H*', call_user_func($hashAlgorithm, ($key^$ipad) . $input)));
}
return $hmac;
}
/**
* Takes comma-separated lists and arrays and removes all duplicates
* If a value in the list is trim(empty), the value is ignored.
......
......@@ -3403,6 +3403,7 @@ class t3lib_TCEforms {
$params['formName'] = $this->formName;
$params['itemName'] = $itemName;
$params['fieldChangeFunc'] = $fieldChangeFunc;
$params['fieldChangeFuncHash'] = t3lib_div::hmac(serialize($fieldChangeFunc));
switch((string)$wConf['type']) {
case 'popup':
......
......@@ -869,6 +869,9 @@ class browse_links {
if ($this->mode=='wizard') { // Functions used, if the link selector is in wizard mode (= TCEforms fields)
if (!$this->areFieldChangeFunctionsValid()) {
$this->P['fieldChangeFunc'] = array();
}
unset($this->P['fieldChangeFunc']['alert']);
reset($this->P['fieldChangeFunc']);
$update='';
......@@ -2345,6 +2348,19 @@ class browse_links {
return $code;
}
/**
* Determines whether submitted field change functions are valid
* and are coming from the system and not from an external abuse.
*
* @return boolean Whether the submitted field change functions are valid
*/
function areFieldChangeFunctionsValid() {
return (
isset($this->P['fieldChangeFunc']) && is_array($this->P['fieldChangeFunc']) && isset($this->P['fieldChangeFuncHash'])
&& $this->P['fieldChangeFuncHash'] == t3lib_div::hmac(serialize($this->P['fieldChangeFunc']))
);
}
}
// Include extension?
......
......@@ -390,6 +390,9 @@ class tx_rtehtmlarea_browse_links extends browse_links {
';
if ($this->mode=='wizard') { // Functions used, if the link selector is in wizard mode (= TCEforms fields)
if (!$this->areFieldChangeFunctionsValid()) {
$this->P['fieldChangeFunc'] = array();
}
unset($this->P['fieldChangeFunc']['alert']);
reset($this->P['fieldChangeFunc']);
$update='';
......
......@@ -78,6 +78,7 @@ class SC_wizard_colorpicker {
var $P; // Wizard parameters, coming from TCEforms linking to the wizard.
var $colorValue; // Value of the current color picked.
var $fieldChangeFunc; // Serialized functions for changing the field... Necessary to call when the value is transferred to the TCEform since the form might need to do internal processing. Otherwise the value is simply not be saved.
var $fieldChangeFuncHash;
var $fieldName; // Form name (from opener script)
var $formName; // Field name (from opener script)
var $md5ID; // ID of element in opener script for which to set color.
......@@ -109,6 +110,7 @@ class SC_wizard_colorpicker {
// Setting GET vars (used in colorpicker script):
$this->colorValue = t3lib_div::_GP('colorValue');
$this->fieldChangeFunc = t3lib_div::_GP('fieldChangeFunc');
$this->fieldChangeFuncHash = t3lib_div::_GP('fieldChangeFuncHash');
$this->fieldName = t3lib_div::_GP('fieldName');
$this->formName = t3lib_div::_GP('formName');
$this->md5ID = t3lib_div::_GP('md5ID');
......@@ -127,7 +129,7 @@ class SC_wizard_colorpicker {
// Setting field-change functions:
$fieldChangeFuncArr = unserialize($this->fieldChangeFunc);
$update = '';
if (is_array($fieldChangeFuncArr)) {
if ($this->areFieldChangeFunctionsValid()) {
unset($fieldChangeFuncArr['alert']);
foreach($fieldChangeFuncArr as $v) {
$update.= '
......@@ -264,7 +266,8 @@ class SC_wizard_colorpicker {
'&formName='.rawurlencode($this->P['formName']).
'&exampleImg='.rawurlencode($this->P['exampleImg']).
'&md5ID='.rawurlencode($this->P['md5ID']).
'&fieldChangeFunc='.rawurlencode(serialize($this->P['fieldChangeFunc']));
'&fieldChangeFunc='.rawurlencode(serialize($this->P['fieldChangeFunc'])) .
'&fieldChangeFuncHash=' . $this->P['fieldChangeFuncHash'];
$this->content.='
<frameset rows="*,1" framespacing="0" frameborder="0" border="0">
......@@ -430,6 +433,19 @@ class SC_wizard_colorpicker {
$hex = implode('',$hexvalue);
return $hex;
}
/**
* Determines whether submitted field change functions are valid
* and are coming from the system and not from an external abuse.
*
* @return boolean Whether the submitted field change functions are valid
*/
function areFieldChangeFunctionsValid() {
return (
$this->fieldChangeFunc && $this->fieldChangeFuncHash
&& $this->fieldChangeFuncHash == t3lib_div::hmac($this->fieldChangeFunc)
);
}
}
// Include extension?
......
......@@ -159,7 +159,9 @@ class SC_wizard_tsconfig {
$this->objString = t3lib_div::_GP('objString');
$this->onlyProperty = t3lib_div::_GP('onlyProperty');
// Preparing some JavaScript code:
if (!is_array($this->P['fieldChangeFunc'])) $this->P['fieldChangeFunc']=array();
if (!$this->areFieldChangeFunctionsValid()) {
$this->P['fieldChangeFunc']=array();
}
unset($this->P['fieldChangeFunc']['alert']);
$update='';
foreach($this->P['fieldChangeFunc'] as $k=>$v) {
......@@ -632,6 +634,19 @@ class SC_wizard_tsconfig {
// Return link:
return $out;
}
/**
* Determines whether submitted field change functions are valid
* and are coming from the system and not from an external abuse.
*
* @return boolean Whether the submitted field change functions are valid
*/
function areFieldChangeFunctionsValid() {
return (
isset($this->P['fieldChangeFunc']) && is_array($this->P['fieldChangeFunc']) && isset($this->P['fieldChangeFuncHash'])
&& $this->P['fieldChangeFuncHash'] == t3lib_div::hmac(serialize($this->P['fieldChangeFunc']))
);
}
}
// Include extension?
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment