Skip to content

Commit 6ed1757

Browse files
AngelFQCAngelFQC
committed
Security: FormValidator check the token even when there are no errors
1 parent 702672c commit 6ed1757

File tree

1 file changed

+89
-86
lines changed

1 file changed

+89
-86
lines changed

Diff for: ‎main/inc/lib/pear/HTML/QuickForm.php

+89-86
Original file line numberDiff line numberDiff line change
@@ -1417,116 +1417,119 @@ function getRequiredNote()
14171417
*/
14181418
public function validate()
14191419
{
1420-
if (count($this->_rules) == 0 && count($this->_formRules) == 0 && $this->isSubmitted()) {
1421-
return (0 == count($this->_errors));
1422-
} elseif (!$this->isSubmitted()) {
1423-
1420+
if (!$this->isSubmitted()) {
14241421
return false;
14251422
}
14261423

1427-
if (null !== $this->getToken()) {
1428-
$check = Security::check_token('form', $this);
1429-
Security::clear_token();
1430-
if (false === $check) {
1431-
// Redirect to the same URL + show token not validated message.
1432-
$url = $this->getAttribute('action');
1433-
Display::addFlash(Display::return_message(get_lang('NotValidated'), 'warning'));
1434-
api_location($url);
1435-
1436-
return false;
1437-
}
1438-
}
1439-
1440-
$registry =& HTML_QuickForm_RuleRegistry::singleton();
1424+
if (count($this->_rules) > 0 || count($this->_formRules) > 0) {
1425+
$registry =& HTML_QuickForm_RuleRegistry::singleton();
14411426

1442-
foreach ($this->_rules as $target => $rules) {
1443-
$submitValue = $this->getSubmitValue($target);
1427+
foreach ($this->_rules as $target => $rules) {
1428+
$submitValue = $this->getSubmitValue($target);
14441429

1445-
foreach ($rules as $rule) {
1446-
if ((isset($rule['group']) && isset($this->_errors[$rule['group']])) ||
1447-
isset($this->_errors[$target])) {
1448-
continue 2;
1449-
}
1450-
// If element is not required and is empty, we shouldn't validate it
1451-
if (!$this->isElementRequired($target)) {
1452-
if (!isset($submitValue) || '' == $submitValue) {
1430+
foreach ($rules as $rule) {
1431+
if ((isset($rule['group']) && isset($this->_errors[$rule['group']])) ||
1432+
isset($this->_errors[$target])) {
14531433
continue 2;
1454-
// Fix for bug #3501: we shouldn't validate not uploaded files, either.
1455-
// Unfortunately, we can't just use $element->isUploadedFile() since
1456-
// the element in question can be buried in group. Thus this hack.
1457-
// See also bug #12014, we should only consider a file that has
1458-
// status UPLOAD_ERR_NO_FILE as not uploaded, in all other cases
1459-
// validation should be performed, so that e.g. 'maxfilesize' rule
1460-
// will display an error if status is UPLOAD_ERR_INI_SIZE
1461-
// or UPLOAD_ERR_FORM_SIZE
1462-
} elseif (is_array($submitValue)) {
1463-
if (false === ($pos = strpos($target, '['))) {
1464-
$isUpload = !empty($this->_submitFiles[$target]);
1465-
} else {
1466-
$base = str_replace(
1467-
array('\\', '\''), array('\\\\', '\\\''),
1468-
substr($target, 0, $pos)
1469-
);
1470-
$idx = "['" . str_replace(
1471-
array('\\', '\'', ']', '['), array('\\\\', '\\\'', '', "']['"),
1472-
substr($target, $pos + 1, -1)
1473-
) . "']";
1474-
eval("\$isUpload = isset(\$this->_submitFiles['{$base}']['name']{$idx});");
1475-
}
1476-
if ($isUpload && (!isset($submitValue['error']) || UPLOAD_ERR_NO_FILE == $submitValue['error'])) {
1434+
}
1435+
// If element is not required and is empty, we shouldn't validate it
1436+
if (!$this->isElementRequired($target)) {
1437+
if (!isset($submitValue) || '' == $submitValue) {
14771438
continue 2;
1439+
// Fix for bug #3501: we shouldn't validate not uploaded files, either.
1440+
// Unfortunately, we can't just use $element->isUploadedFile() since
1441+
// the element in question can be buried in group. Thus this hack.
1442+
// See also bug #12014, we should only consider a file that has
1443+
// status UPLOAD_ERR_NO_FILE as not uploaded, in all other cases
1444+
// validation should be performed, so that e.g. 'maxfilesize' rule
1445+
// will display an error if status is UPLOAD_ERR_INI_SIZE
1446+
// or UPLOAD_ERR_FORM_SIZE
1447+
} elseif (is_array($submitValue)) {
1448+
if (false === ($pos = strpos($target, '['))) {
1449+
$isUpload = !empty($this->_submitFiles[$target]);
1450+
} else {
1451+
$base = str_replace(
1452+
array('\\', '\''), array('\\\\', '\\\''),
1453+
substr($target, 0, $pos)
1454+
);
1455+
$idx = "['" . str_replace(
1456+
array('\\', '\'', ']', '['), array('\\\\', '\\\'', '', "']['"),
1457+
substr($target, $pos + 1, -1)
1458+
) . "']";
1459+
eval("\$isUpload = isset(\$this->_submitFiles['{$base}']['name']{$idx});");
1460+
}
1461+
if ($isUpload && (!isset($submitValue['error']) || UPLOAD_ERR_NO_FILE == $submitValue['error'])) {
1462+
continue 2;
1463+
}
14781464
}
14791465
}
1480-
}
14811466

1482-
if (isset($rule['dependent'])) {
1483-
$values = array($submitValue);
1484-
if (is_array($rule['dependent'])) {
1485-
foreach ($rule['dependent'] as $elName) {
1486-
$values[] = $this->getSubmitValue($elName);
1467+
if (isset($rule['dependent'])) {
1468+
$values = array($submitValue);
1469+
if (is_array($rule['dependent'])) {
1470+
foreach ($rule['dependent'] as $elName) {
1471+
$values[] = $this->getSubmitValue($elName);
1472+
}
1473+
} else {
1474+
$values[] = $rule['dependent'];
14871475
}
1476+
$result = $registry->validate(
1477+
$rule['type'],
1478+
$values,
1479+
$rule['format'],
1480+
true
1481+
);
1482+
} elseif (is_array($submitValue) && !isset($rule['howmany'])) {
1483+
$result = $registry->validate($rule['type'], $submitValue, $rule['format'], true);
14881484
} else {
1489-
$values[] = $rule['dependent'];
1485+
$result = $registry->validate(
1486+
$rule['type'],
1487+
$submitValue,
1488+
$rule['format'],
1489+
false
1490+
);
1491+
}
1492+
1493+
if (!$result || (!empty($rule['howmany']) && $rule['howmany'] > (int)$result)) {
1494+
if (isset($rule['group'])) {
1495+
$this->_errors[$rule['group']] = $rule['message'];
1496+
} else {
1497+
$this->_errors[$target] = $rule['message'];
1498+
}
14901499
}
1491-
$result = $registry->validate(
1492-
$rule['type'],
1493-
$values,
1494-
$rule['format'],
1495-
true
1496-
);
1497-
} elseif (is_array($submitValue) && !isset($rule['howmany'])) {
1498-
$result = $registry->validate($rule['type'], $submitValue, $rule['format'], true);
1499-
} else {
1500-
$result = $registry->validate(
1501-
$rule['type'],
1502-
$submitValue,
1503-
$rule['format'],
1504-
false
1505-
);
15061500
}
1501+
}
15071502

1508-
if (!$result || (!empty($rule['howmany']) && $rule['howmany'] > (int)$result)) {
1509-
if (isset($rule['group'])) {
1510-
$this->_errors[$rule['group']] = $rule['message'];
1503+
// process the global rules now
1504+
foreach ($this->_formRules as $rule) {
1505+
if (true !== ($res = call_user_func($rule, $this->_submitValues, $this->_submitFiles))) {
1506+
if (is_array($res)) {
1507+
$this->_errors += $res;
15111508
} else {
1512-
$this->_errors[$target] = $rule['message'];
1509+
throw new \Exception('Form rule callback returned invalid value in HTML_QuickForm::validate()');
15131510
}
15141511
}
15151512
}
1513+
1514+
if (count($this->_errors) > 0) {
1515+
return false;
1516+
}
15161517
}
15171518

1518-
// process the global rules now
1519-
foreach ($this->_formRules as $rule) {
1520-
if (true !== ($res = call_user_func($rule, $this->_submitValues, $this->_submitFiles))) {
1521-
if (is_array($res)) {
1522-
$this->_errors += $res;
1523-
} else {
1524-
throw new \Exception('Form rule callback returned invalid value in HTML_QuickForm::validate()');
1525-
}
1519+
if (null !== $this->getToken()) {
1520+
$check = Security::check_token('form', $this);
1521+
Security::clear_token();
1522+
if (false === $check) {
1523+
// Redirect to the same URL + show token not validated message.
1524+
$url = $this->getAttribute('action');
1525+
Display::addFlash(Display::return_message(get_lang('NotValidated'), 'warning'));
1526+
api_location($url);
1527+
1528+
return false;
15261529
}
15271530
}
15281531

1529-
return (0 == count($this->_errors));
1532+
return true;
15301533
}
15311534

15321535
/**

0 commit comments

Comments
 (0)