Add in new setting caspreparse.

.travis.yml

@@ -92,6 +92,7 @@ install:
   - moodle-plugin-ci add-config 'define("QTYPE_STACK_TEST_CONFIG_MAXIMAVERSION",   "'`maxima --version | sed 's/Maxima //'`'");'
   - moodle-plugin-ci add-config 'define("QTYPE_STACK_TEST_CONFIG_CASTIMEOUT",      "10");'
   - moodle-plugin-ci add-config 'define("QTYPE_STACK_TEST_CONFIG_CASRESULTSCACHE", "db");'
+  - moodle-plugin-ci add-config 'define("QTYPE_STACK_TEST_CONFIG_CASPREPARSE", "true");'
   - moodle-plugin-ci add-config 'define("QTYPE_STACK_TEST_CONFIG_MAXIMACOMMAND",   "");'
   - moodle-plugin-ci add-config 'define("QTYPE_STACK_TEST_CONFIG_MAXIMACOMMANDOPT",   "timeout --kill-after=10s 10s /home/travis/build/maxima_opt_auto -eval '\''(cl-user::run)'\''");'
   - moodle-plugin-ci add-config 'define("QTYPE_STACK_TEST_CONFIG_MAXIMACOMMANDSERVER",   "http://pool.home:8080/MaximaPool/MaximaPool");'

db/install.php

@@ -60,6 +60,7 @@ function xmldb_qtype_stack_install() {
         set_config('maximaversion', 'default', 'qtype_stack');
         set_config('castimeout', 10, 'qtype_stack');
         set_config('casresultscache', 'db', 'qtype_stack');
+        set_config('caspreparse', 'true', 'qtype_stack');
         set_config('maximacommand', '', 'qtype_stack');
         set_config('maximacommandopt', '', 'qtype_stack');
         set_config('maximacommandserver', '', 'qtype_stack');

doc/en/Developer/Development_track.md

@@ -16,6 +16,8 @@ Mostly minor bug fixes.
 * Done. Add in warnings of language mismatch in parts of a question.
 * Done. Add in warnings where the answer test needs a raw input but appears to get a calculated value.
 * Done. Expand `rand` to accept sets and make a random selection from sets.  Add `rand_selection_with_replacement`.
+* Done. Improve checking of teacher's code for better security, this requires a new admin setting `caspreparse` for back compatibility.
+
 
 ## Version 4.4
 

doc/en/Developer/Unit_tests.md

@@ -44,6 +44,7 @@ Other options for the platform are `linux` and `linux-optimised`.
     define('QTYPE_STACK_TEST_CONFIG_PLOTCOMMAND',     '');
 
     define('QTYPE_STACK_TEST_CONFIG_CASRESULTSCACHE', 'db');
+    define('QTYPE_STACK_TEST_CONFIG_CASPREPARSE', 'true');
 
 You should probably copy the settings from Admin -> Plugins -> Question types -> STACK.
 However, you can use the flexibility to have different configurations of STACK

lang/en/qtype_stack.php

@@ -408,6 +408,10 @@ $string['settingcasresultscache_desc'] = 'This setting determines whether calls
 $string['settingcasresultscache_none'] = 'Do not cache';
 $string['settingcastimeout'] = 'CAS connection timeout';
 $string['settingcastimeout_desc'] = 'The timout to use when trying to connect to Maxima.';
+$string['settingcaspreparse'] = 'Pre-parse all code before sending to Maxima.';
+$string['settingcaspreparse_desc'] = 'We recommend all code in question variables, etc., even from trusted teachers, is pre-parsed for potentially malicious patterns.  This is especially important when accepting imported questions from known sources.  However, it is possible for this pre-parse to time-out and it would be necessary to disable this check (temporarily) to back out of a potential dead end.  This code is still under testing and development and this setting will be removed in future releases ensuring this pre-parse is always applied.';
+$string['settingcaspreparse_true'] = 'Always preparse';
+$string['settingcaspreparse_false'] = 'Do not preparse (not recommended)';
 $string['settingdefaultinputoptions'] = 'Default input options';
 $string['settingdefaultinputoptions_desc'] = 'Used when creating a new question, or adding a new input to an existing question.';
 $string['settingdefaultquestionoptions'] = 'Default input options';

settings.php

@@ -85,6 +85,13 @@ $settings->add(new admin_setting_configselect('qtype_stack/casresultscache',
             'db' => get_string('settingcasresultscache_db', 'qtype_stack'),
         )));
 
+$settings->add(new admin_setting_configselect('qtype_stack/caspreparse',
+        get_string('settingcaspreparse', 'qtype_stack'),
+        get_string('settingcaspreparse_desc', 'qtype_stack'), 'true', array(
+            'true' => get_string('settingcaspreparse_true', 'qtype_stack'),
+            'false' => get_string('settingcaspreparse_false', 'qtype_stack'),
+        )));
+
 $settings->add(new admin_setting_configtext('qtype_stack/maximacommand',
         get_string('settingplatformmaximacommand', 'qtype_stack'),
         get_string('settingplatformmaximacommand_desc', 'qtype_stack'), null));

stack/cas/keyval.class.php

@@ -19,6 +19,7 @@ defined('MOODLE_INTERNAL') || die();
 require_once(__DIR__ . '/../maximaparser/utils.php');
 require_once(__DIR__ . '/../maximaparser/MP_classes.php');
 require_once(__DIR__ . '/cassession2.class.php');
+require_once(__DIR__ . '/../utils.class.php');
 
 /**
  * Class to parse user-entered data into CAS sessions.
@@ -114,8 +115,12 @@ class stack_cas_keyval {
 
         $ast = maxima_parser_utils::strip_comments($ast);
 
+        $vallist = array();
         // Update the types and values for future insert-stars and other logic.
+        $config = stack_utils::get_config();
+        if ($config->caspreparse == 'true') {
             $vallist = maxima_parser_utils::identify_identifier_values($ast, $this->security->get_context());
+        }
         if (isset($vallist['% TIMEOUT %'])) {
             $this->errors[] = stack_string('stackCas_overlyComplexSubstitutionGraphOrRandomisation');
             $this->valid = false;

stack/maxima/stackmaxima.mac

@@ -3094,5 +3094,5 @@ load(linearalgebra);
 
 /* Stack expects some output with the version number the output happens at */
 /* maximalocal.mac after additional library loading */
-stackmaximaversion:2021010100$
+stackmaximaversion:2021071600$
 

stack/maximaparser/utils.php

@@ -331,7 +331,9 @@ class maxima_parser_utils {
                     }
                 }
                 // Algsys needs the list contents protected.
-                if ($node->parentnode instanceof MP_List && $node->parentnode->parentnode !== null && $node->parentnode->parentnode instanceof MP_FunctionCall && $node->parentnode->parentnode->name instanceof MP_Atom
+                if ($node->parentnode instanceof MP_List && $node->parentnode->parentnode !== null
+                        && $node->parentnode->parentnode instanceof MP_FunctionCall
+                        && $node->parentnode->parentnode->name instanceof MP_Atom
                         && $node->parentnode->parentnode->name->value === 'algsys') {
                     if ($node->argument_of('algsys') === 1) {
                         return true;
@@ -467,7 +469,9 @@ class maxima_parser_utils {
                     }
                 }
                 // Algsys needs the list contents protected.
-                if ($node->parentnode instanceof MP_List && $node->parentnode->parentnode !== null && $node->parentnode->parentnode instanceof MP_FunctionCall && $node->parentnode->parentnode->name instanceof MP_Atom
+                if ($node->parentnode instanceof MP_List && $node->parentnode->parentnode !== null
+                        && $node->parentnode->parentnode instanceof MP_FunctionCall
+                        && $node->parentnode->parentnode->name instanceof MP_Atom
                         && $node->parentnode->parentnode->name->value === 'algsys') {
                     if ($node->argument_of('algsys') === 1) {
                         return true;
@@ -1420,7 +1424,8 @@ class maxima_parser_utils {
                 if (!isset($types['MP_Statement']) && $types['totalnodes'] > $mergelimit
                         && count($types['vars']) + 1 < $mergelimit && !isset($types['ops'][':'])
                         && !isset($types['ops']['=']) && !isset($types['funs']['subst'])
-                        && !isset($types['funs']['ev']) && !isset($types['funs']['solve']) && !isset($types['funs']['algsys']) && !isset($types['funs']['at'])) {
+                        && !isset($types['funs']['ev']) && !isset($types['funs']['solve'])
+                        && !isset($types['funs']['algsys']) && !isset($types['funs']['at'])) {
                     $ids = [];
                     foreach ($types['vars'] as $key => $value) {
                         $ids[] = new MP_Identifier($key);
@@ -1559,7 +1564,8 @@ class maxima_parser_utils {
                     continue;
                 }
                 $types = $value->type_count();
-                if ($types['has control flow'] || isset($types['funs']['solve']) || isset($types['funs']['algsys']) || isset($types['funs']['ev'])
+                if ($types['has control flow'] || isset($types['funs']['solve']) || isset($types['funs']['algsys'])
+                        || isset($types['funs']['ev'])
                         || isset($types['funs']['subst']) || isset($types['funs']['at']) || isset($types['ops'][':'])) {
                     $t = new MP_Operation(':', new MP_Identifier($key), $value);
                     $workset2[$t->toString()] = $t;
@@ -1812,7 +1818,8 @@ class maxima_parser_utils {
                     continue;
                 }
                 $types = $value->type_count();
-                if ($types['has control flow'] || isset($types['funs']['solve']) || isset($types['funs']['algsys']) || isset($types['funs']['ev'])
+                if ($types['has control flow'] || isset($types['funs']['solve']) || isset($types['funs']['algsys'])
+                        || isset($types['funs']['ev'])
                         || isset($types['funs']['subst']) || isset($types['funs']['at'])) {
                     $t = new MP_Operation(':', new MP_Identifier($key), $value);
                     $workset3[$t->toString()] = $t;

tests/fixtures/test_maxima_configuration.php

@@ -70,6 +70,7 @@ abstract class qtype_stack_test_config {
         set_config('maximaversion',       QTYPE_STACK_TEST_CONFIG_MAXIMAVERSION,       'qtype_stack');
         set_config('castimeout',          QTYPE_STACK_TEST_CONFIG_CASTIMEOUT,          'qtype_stack');
         set_config('casresultscache',     QTYPE_STACK_TEST_CONFIG_CASRESULTSCACHE,     'qtype_stack');
+        set_config('caspreparse',         QTYPE_STACK_TEST_CONFIG_CASPREPARSE,         'qtype_stack');
         set_config('maximacommand',       QTYPE_STACK_TEST_CONFIG_MAXIMACOMMAND,       'qtype_stack');
         set_config('maximacommandopt',    QTYPE_STACK_TEST_CONFIG_MAXIMACOMMANDOPT,    'qtype_stack');
         set_config('maximacommandserver', QTYPE_STACK_TEST_CONFIG_MAXIMACOMMANDSERVER, 'qtype_stack');

version.php

@@ -24,7 +24,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2021010100;
+$plugin->version   = 2021071600;
 $plugin->requires  = 2018051700;
 $plugin->cron      = 0;
 $plugin->component = 'qtype_stack';