diff --git a/question/type/calculated/edit_calculated_form.php b/question/type/calculated/edit_calculated_form.php
index d6d9f450e90..c3c02a90533 100644
--- a/question/type/calculated/edit_calculated_form.php
+++ b/question/type/calculated/edit_calculated_form.php
@@ -212,36 +212,39 @@ class qtype_calculated_edit_form extends qtype_numerical_edit_form {
return 'calculated';
}
+ /**
+ * Validate the equations in the some question content.
+ * @param array $errors where errors are being accumulated.
+ * @param string $field the field being validated.
+ * @param string $text the content of that field.
+ * @return array the updated $errors array.
+ */
+ protected function validate_text($errors, $field, $text) {
+ $problems = qtype_calculated_find_formula_errors_in_text($text);
+ if ($problems) {
+ $errors[$field] = $problems;
+ }
+ return $errors;
+ }
+
public function validation($data, $files) {
+ $errors = parent::validation($data, $files);
// Verifying for errors in {=...} in question text.
- $qtext = "";
- $qtextremaining = $data['questiontext']['text'];
- $possibledatasets = $this->qtypeobj->find_dataset_names($data['questiontext']['text']);
- foreach ($possibledatasets as $name => $value) {
- $qtextremaining = str_replace('{'.$name.'}', '1', $qtextremaining);
- }
- while (preg_match('~\{=([^[:space:]}]*)}~', $qtextremaining, $regs1)) {
- $qtextsplits = explode($regs1[0], $qtextremaining, 2);
- $qtext = $qtext.$qtextsplits[0];
- $qtextremaining = $qtextsplits[1];
- if (!empty($regs1[1]) && $formulaerrors =
- qtype_calculated_find_formula_errors($regs1[1])) {
- if (!isset($errors['questiontext'])) {
- $errors['questiontext'] = $formulaerrors.':'.$regs1[1];
- } else {
- $errors['questiontext'] .= '
'.$formulaerrors.':'.$regs1[1];
- }
- }
- }
-
- $errors = parent::validation($data, $files);
+ $errors = $this->validate_text($errors, 'questiontext', $data['questiontext']['text']);
+ $errors = $this->validate_text($errors, 'generalfeedback', $data['generalfeedback']['text']);
// Check that the answers use datasets.
$answers = $data['answer'];
$mandatorydatasets = array();
foreach ($answers as $key => $answer) {
+ $problems = qtype_calculated_find_formula_errors($answer);
+ if ($problems) {
+ $errors['answeroptions['.$key.']'] = $problems;
+ }
$mandatorydatasets += $this->qtypeobj->find_dataset_names($answer);
+ $errors = $this->validate_text($errors, 'feedback[' . $key . ']',
+ $data['feedback'][$key]['text']);
}
if (empty($mandatorydatasets)) {
foreach ($answers as $key => $answer) {
diff --git a/question/type/calculated/question.php b/question/type/calculated/question.php
index 71b25a6790b..92ad9e79ac8 100644
--- a/question/type/calculated/question.php
+++ b/question/type/calculated/question.php
@@ -278,6 +278,7 @@ class qtype_calculated_dataset_loader {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_calculated_variable_substituter {
+
/** @var array variable name => value */
protected $values;
@@ -465,108 +466,11 @@ class qtype_calculated_variable_substituter {
* @return string the text with values substituted.
*/
public function replace_expressions_in_text($text, $length = null, $format = null) {
- $vs = $this; // Can't see to use $this in a PHP closure.
- $text = preg_replace_callback('~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~',
+ $vs = $this; // Can't use $this in a PHP closure.
+ $text = preg_replace_callback(qtype_calculated::FORMULAS_IN_TEXT_REGEX,
function ($matches) use ($vs, $format, $length) {
return $vs->format_float($vs->calculate($matches[1]), $length, $format);
}, $text);
return $this->substitute_values_pretty($text);
}
-
- /**
- * Return an array describing any problems there are with an expression.
- * Returns false if the expression is fine.
- * @param string $formula an expression.
- * @return array|false list of problems, or false if the exression is OK.
- */
- public function get_formula_errors($formula) {
- // Validates the formula submitted from the question edit page.
- // Returns false if everything is alright
- // otherwise it constructs an error message.
- // Strip away dataset names.
- while (preg_match('~\\{[[:alpha:]][^>} <{"\']*\\}~', $formula, $regs)) {
- $formula = str_replace($regs[0], '1', $formula);
- }
-
- // Strip away empty space and lowercase it.
- $formula = strtolower(str_replace(' ', '', $formula));
-
- $safeoperatorchar = '-+/*%>:^\~=&|!'; /* */
- $operatorornumber = "[$safeoperatorchar.0-9eE]";
-
- while (preg_match("~(^|[$safeoperatorchar,(])([a-z0-9_]*)" .
- "\\(($operatorornumber+(,$operatorornumber+((,$operatorornumber+)+)?)?)?\\)~",
- $formula, $regs)) {
- switch ($regs[2]) {
- // Simple parenthesis.
- case '':
- if ((isset($regs[4]) && $regs[4]) || strlen($regs[3]) == 0) {
- return get_string('illegalformulasyntax', 'qtype_calculated', $regs[0]);
- }
- break;
-
- // Zero argument functions.
- case 'pi':
- if ($regs[3]) {
- return get_string('functiontakesnoargs', 'qtype_calculated', $regs[2]);
- }
- break;
-
- // Single argument functions (the most common case).
- case 'abs': case 'acos': case 'acosh': case 'asin': case 'asinh':
- case 'atan': case 'atanh': case 'bindec': case 'ceil': case 'cos':
- case 'cosh': case 'decbin': case 'decoct': case 'deg2rad':
- case 'exp': case 'expm1': case 'floor': case 'is_finite':
- case 'is_infinite': case 'is_nan': case 'log10': case 'log1p':
- case 'octdec': case 'rad2deg': case 'sin': case 'sinh': case 'sqrt':
- case 'tan': case 'tanh':
- if (!empty($regs[4]) || empty($regs[3])) {
- return get_string('functiontakesonearg', 'qtype_calculated', $regs[2]);
- }
- break;
-
- // Functions that take one or two arguments.
- case 'log': case 'round':
- if (!empty($regs[5]) || empty($regs[3])) {
- return get_string('functiontakesoneortwoargs', 'qtype_calculated',
- $regs[2]);
- }
- break;
-
- // Functions that must have two arguments.
- case 'atan2': case 'fmod': case 'pow':
- if (!empty($regs[5]) || empty($regs[4])) {
- return get_string('functiontakestwoargs', 'qtype_calculated', $regs[2]);
- }
- break;
-
- // Functions that take two or more arguments.
- case 'min': case 'max':
- if (empty($regs[4])) {
- return get_string('functiontakesatleasttwo', 'qtype_calculated', $regs[2]);
- }
- break;
-
- default:
- return get_string('unsupportedformulafunction', 'qtype_calculated', $regs[2]);
- }
-
- // Exchange the function call with '1' and then check for another function call.
-
- if ($regs[1]) {
- // The function call is proceeded by an operator.
- $formula = str_replace($regs[0], $regs[1] . '1', $formula);
- } else {
- // The function call starts the formula.
- $formula = preg_replace("~^$regs[2]\\([^)]*\\)~", '1', $formula);
- }
- }
-
- if (preg_match("~[^$safeoperatorchar.0-9eE]+~", $formula, $regs)) {
- return get_string('illegalformulasyntax', 'qtype_calculated', $regs[0]);
- } else {
- // Formula just might be valid.
- return false;
- }
- }
-}
\ No newline at end of file
+}
diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
index 75a047e9a30..d77ff2f59c4 100644
--- a/question/type/calculated/questiontype.php
+++ b/question/type/calculated/questiontype.php
@@ -37,6 +37,9 @@ require_once($CFG->dirroot . '/question/type/numerical/question.php');
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_calculated extends question_type {
+ /** Regular expression that finds the formulas in content. */
+ const FORMULAS_IN_TEXT_REGEX = '~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)\}~';
+
const MAX_DATASET_ITEMS = 100;
public $wizardpagesnumber = 3;
@@ -481,6 +484,36 @@ class qtype_calculated extends question_type {
$mform->display();
}
+ /**
+ * Verify that the equations in part of the question are OK.
+ * We throw an exception here because this should have already been validated
+ * by the form. This is just a last line of defence to prevent a question
+ * being stored in the database if it has bad formulas. This saves us from,
+ * for example, malicious imports.
+ * @param string $text containing equations.
+ */
+ protected function validate_text($text) {
+ $error = qtype_calculated_find_formula_errors_in_text($text);
+ if ($error) {
+ throw new coding_exception($error);
+ }
+ }
+
+ /**
+ * Verify that an answer is OK.
+ * We throw an exception here because this should have already been validated
+ * by the form. This is just a last line of defence to prevent a question
+ * being stored in the database if it has bad formulas. This saves us from,
+ * for example, malicious imports.
+ * @param string $text containing equations.
+ */
+ protected function validate_answer($answer) {
+ $error = qtype_calculated_find_formula_errors($answer);
+ if ($error) {
+ throw new coding_exception($error);
+ }
+ }
+
/**
* This method prepare the $datasets in a format similar to dadatesetdefinitions_form.php
* so that they can be saved
@@ -494,11 +527,12 @@ class qtype_calculated extends question_type {
* @param int $questionfromid default = '0'
*/
public function preparedatasets($form , $questionfromid = '0') {
+
// The dataset names present in the edit_question_form and edit_calculated_form
// are retrieved.
$possibledatasets = $this->find_dataset_names($form->questiontext);
$mandatorydatasets = array();
- foreach ($form->answers as $answer) {
+ foreach ($form->answers as $key => $answer) {
$mandatorydatasets += $this->find_dataset_names($answer);
}
// If there are identical datasetdefs already saved in the original question
@@ -587,8 +621,15 @@ class qtype_calculated extends question_type {
*/
public function save_question($question, $form) {
global $DB;
+
+ if (isset($form->correctfeedback)) {
+ $this->validate_text($form->correctfeedback['text']);
+ $this->validate_text($form->partiallycorrectfeedback['text']);
+ $this->validate_text($form->incorrectfeedback['text']);
+ }
+
if ($this->wizardpagesnumber() == 1 || $question->qtype == 'calculatedsimple') {
- $question = parent::save_question($question, $form);
+ $question = parent::save_question($question, $form);
return $question;
}
@@ -607,6 +648,14 @@ class qtype_calculated extends question_type {
case '' :
case 'question': // Coming from the first page, creating the second.
if (empty($form->id)) { // or a new question $form->id is empty.
+ // Make it impossible to save bad formulas anywhere.
+ $this->validate_text($form->questiontext['text']);
+ $this->validate_text($form->generalfeedback['text']);
+ foreach ($form->answer as $key => $answer) {
+ $this->validate_answer($answer);
+ $this->validate_text($form->feedback[$key]['text']);
+ }
+
$question = parent::save_question($question, $form);
// Prepare the datasets using default $questionfromid.
$this->preparedatasets($form);
@@ -1899,6 +1948,11 @@ function qtype_calculated_calculate_answer($formula, $individualdata,
}
+/**
+ * Validate a forumula.
+ * @param string $formula the formula to validate.
+ * @return string|boolean false if there are no problems. Otherwise a string error message.
+ */
function qtype_calculated_find_formula_errors($formula) {
// Validates the formula submitted from the question edit page.
// Returns false if everything is alright
@@ -1927,7 +1981,7 @@ function qtype_calculated_find_formula_errors($formula) {
// Zero argument functions.
case 'pi':
- if ($regs[3]) {
+ if (array_key_exists(3, $regs)) {
return get_string('functiontakesnoargs', 'qtype_calculated', $regs[2]);
}
break;
@@ -1988,3 +2042,26 @@ function qtype_calculated_find_formula_errors($formula) {
return false;
}
}
+
+/**
+ * Validate all the forumulas in a bit of text.
+ * @param string $text the text in which to validate the formulas.
+ * @return string|boolean false if there are no problems. Otherwise a string error message.
+ */
+function qtype_calculated_find_formula_errors_in_text($text) {
+ preg_match_all(qtype_calculated::FORMULAS_IN_TEXT_REGEX, $text, $matches);
+
+ $errors = array();
+ foreach ($matches[1] as $match) {
+ $error = qtype_calculated_find_formula_errors($match);
+ if ($error) {
+ $errors[] = $error;
+ }
+ }
+
+ if ($errors) {
+ return implode(' ', $errors);
+ }
+
+ return false;
+}
diff --git a/question/type/calculated/tests/formula_validation_test.php b/question/type/calculated/tests/formula_validation_test.php
new file mode 100644
index 00000000000..d7ff76ed18e
--- /dev/null
+++ b/question/type/calculated/tests/formula_validation_test.php
@@ -0,0 +1,105 @@
+.
+
+/**
+ * Unit tests for formula validation code.
+ *
+ * @package qtype_calculated
+ * @copyright 2014 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot . '/question/type/calculated/questiontype.php');
+
+
+/**
+ * Unit tests for formula validation code.
+ *
+ * @copyright 2014 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class qtype_calculated_formula_validation_testcase extends basic_testcase {
+ protected function assert_nonempty_string($actual) {
+ $this->assertInternalType('string', $actual);
+ $this->assertNotEquals('', $actual);
+ }
+
+ public function test_simple_equations_ok() {
+ $this->assertFalse(qtype_calculated_find_formula_errors(1));
+ $this->assertFalse(qtype_calculated_find_formula_errors('1 + 1'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('{x} + {y}'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('{x}*{y}'));
+ }
+
+ public function test_safe_functions_ok() {
+ $this->assertFalse(qtype_calculated_find_formula_errors('abs(-1)'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('tan(pi())'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('log(10)'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('log(64, 2)'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('atan2(1.0, 1.0)'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0)'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0, 2.0)'));
+ $this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0, 2, 3)'));
+ }
+
+ public function test_dangerous_functions_blocked() {
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('eval(1)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('system(1)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('base64_decode(1)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('unserialize(1)'));
+
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('cos(tan(1) + abs(cos(eval)) * pi())'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('eval (CONSTANTREADASSTRING)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors("eval \t ()"));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('"eval"()'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('?>assert_nonempty_string(qtype_calculated_find_formula_errors('?>assert_nonempty_string(qtype_calculated_find_formula_errors('abs(-1, 1)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('abs()'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('pi(1)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('log()'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('log(64, 2, 3)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('atan2(1.0)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('atan2(1.0, 1.0, 2.0)'));
+ $this->assert_nonempty_string(qtype_calculated_find_formula_errors('max(1.0)'));
+ }
+
+ public function test_validation_of_formulas_in_text_ok() {
+ $this->assertFalse(qtype_calculated_find_formula_errors_in_text(
+ '
Look no equations.
')); + $this->assertFalse(qtype_calculated_find_formula_errors_in_text( + 'Simple variable: {x}.
')); + $this->assertFalse(qtype_calculated_find_formula_errors_in_text( + 'This is an equation: {=1+1}, as is this: {={x}+{y}}.
' . + 'Here is a more complex one: {=sin(2*pi()*{theta})}.
')); + } + + public function test_validation_of_formulas_in_text_bad_function() { + $this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text( + 'This is an equation: {=eval(1)}.
')); + $this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text( + 'Good: {=1+1}, bad: {=eval(1)}, good: {={x}+{y}}.
')); + $this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text( + 'Bad: {=eval(1)}, bad: {=system(1)}.
')); + } +} diff --git a/question/type/calculatedmulti/edit_calculatedmulti_form.php b/question/type/calculatedmulti/edit_calculatedmulti_form.php index eee479f1138..56fce17908b 100644 --- a/question/type/calculatedmulti/edit_calculatedmulti_form.php +++ b/question/type/calculatedmulti/edit_calculatedmulti_form.php @@ -228,30 +228,31 @@ class qtype_calculatedmulti_edit_form extends question_edit_form { return $question; } + /** + * Validate the equations in the some question content. + * @param array $errors where errors are being accumulated. + * @param string $field the field being validated. + * @param string $text the content of that field. + * @return array the updated $errors array. + */ + protected function validate_text($errors, $field, $text) { + $problems = qtype_calculated_find_formula_errors_in_text($text); + if ($problems) { + $errors[$field] = $problems; + } + return $errors; + } + public function validation($data, $files) { $errors = parent::validation($data, $files); // Verifying for errors in {=...} in question text. - $qtext = ''; - $qtextremaining = $data['questiontext']['text']; - $possibledatasets = $this->qtypeobj->find_dataset_names($data['questiontext']['text']); - foreach ($possibledatasets as $name => $value) { - $qtextremaining = str_replace('{'.$name.'}', '1', $qtextremaining); - } + $errors = $this->validate_text($errors, 'questiontext', $data['questiontext']['text']); + $errors = $this->validate_text($errors, 'generalfeedback', $data['generalfeedback']['text']); + $errors = $this->validate_text($errors, 'correctfeedback', $data['correctfeedback']['text']); + $errors = $this->validate_text($errors, 'partiallycorrectfeedback', $data['partiallycorrectfeedback']['text']); + $errors = $this->validate_text($errors, 'incorrectfeedback', $data['incorrectfeedback']['text']); - while (preg_match('~\{=([^[:space:]}]*)}~', $qtextremaining, $regs1)) { - $qtextsplits = explode($regs1[0], $qtextremaining, 2); - $qtext = $qtext.$qtextsplits[0]; - $qtextremaining = $qtextsplits[1]; - if (!empty($regs1[1]) && $formulaerrors = - qtype_calculated_find_formula_errors($regs1[1])) { - if (!isset($errors['questiontext'])) { - $errors['questiontext'] = $formulaerrors.':'.$regs1[1]; - } else { - $errors['questiontext'] .= '