Fix possible serious errors in dynamic form fields

Fix dropping of materialized view when variable name is changed

Ensure view exists before merging updates

Prevent possible sql injection error in field name used in the materialized
view.

Prevent possible xss error in the display of the field label and variable
name in the admin panel.

include/ajax.tickets.php

@@ -211,10 +211,11 @@ class TicketsAjaxAPI extends AjaxController {
         foreach (TicketForm::getInstance()->getFields() as $f) {
             if (isset($req[$f->getFormName()])
                     && ($val = $req[$f->getFormName()])) {
-                $name = $f->get('name') ? $f->get('name') : 'field_'.$f->get('id');
-                $cwhere = "cdata.`$name` LIKE '%".db_real_escape($val)."%'";
+                $name = $f->get('name') ? db_real_escape($f->get('name'))
+                    : 'field_'.$f->get('id');
+                $cwhere = "cdata.\"$name\" LIKE '%".db_real_escape($val)."%'";
                 if ($f->getImpl()->hasIdValue() && is_numeric($val))
-                    $cwhere .= " OR cdata.`{$name}_id` = ".db_input($val);
+                    $cwhere .= " OR cdata.\"{$name}_id\" = ".db_input($val);
                 $where .= ' AND ('.$cwhere.')';
                 $cdata_search = true;
             }

include/class.dynamic_forms.php

@@ -210,15 +210,15 @@ class TicketForm extends DynamicForm {
             if (!$impl->hasData() || $impl->isPresentationOnly())
                 continue;
 
-            $name = ($f->get('name')) ? $f->get('name')
+            $name = ($f->get('name')) ? db_real_escape($f->get('name'))
                 : 'field_'.$f->get('id');
 
             $fields[] = sprintf(
-                'MAX(IF(field.name=\'%1$s\',ans.value,NULL)) as `%1$s`',
+                'MAX(IF(field.name=\'%1$s\',ans.value,NULL)) as "%1$s"',
                 $name);
             if ($impl->hasIdValue()) {
                 $fields[] = sprintf(
-                    'MAX(IF(field.name=\'%1$s\',ans.value_id,NULL)) as `%1$s_id`',
+                    'MAX(IF(field.name=\'%1$s\',ans.value_id,NULL)) as "%1$s_id"',
                     $name);
             }
         }
@@ -261,21 +261,21 @@ class TicketForm extends DynamicForm {
         if (!($e = $answer->getEntry()) || $e->get('object_type') != 'T')
             return;
 
-        // If the `name` column is in the dirty list, we would be renaming a
-        // column. Delete the view instead.
-        if (isset($data['dirty']) && isset($data['dirty']['name']))
-            return self::dropDynamicDataView();
-
         // $record = array();
         // $record[$f] = $answer->value'
         // TicketFormData::objects()->filter(array('ticket_id'=>$a))
         //      ->merge($record);
+        $sql = 'SHOW TABLES LIKE \''.TABLE_PREFIX.'ticket__cdata\'';
+        if (!db_num_rows(db_query($sql)))
+            return;
+
         $f = $answer->getField();
-        $name = $f->get('name') ? $f->get('name') : 'field_'.$f->get('id');
+        $name = $f->get('name') ? db_real_escape($f->get('name'))
+            : 'field_'.$f->get('id');
         $ids = $f->hasIdValue();
-        $fields = sprintf('`%s`=', $name) . db_input($answer->get('value'));
+        $fields = sprintf('"%s"=', $name) . db_input($answer->get('value'));
         if ($f->hasIdValue())
-            $fields .= sprintf(',`%s_id`=', $name) . db_input($answer->getIdValue());
+            $fields .= sprintf(',"%s_id"=', $name) . db_input($answer->getIdValue());
         $sql = 'INSERT INTO `'.TABLE_PREFIX.'ticket__cdata` SET '.$fields
             .', `ticket_id`='.db_input($answer->getEntry()->get('object_id'))
             .' ON DUPLICATE KEY UPDATE '.$fields;
@@ -309,6 +309,13 @@ Signal::connect('model.deleted',
     array('TicketForm', 'dropDynamicDataView'),
     'DynamicFormField',
     function($o) { return $o->getForm()->get('type') == 'T'; });
+// If the `name` column is in the dirty list, we would be renaming a
+// column. Delete the view instead.
+Signal::connect('model.updated',
+    array('TicketForm', 'dropDynamicDataView'),
+    'DynamicFormField',
+    // TODO: Lookup the dynamic form to verify {type == 'T'}
+    function($o, $d) { return isset($d['dirty']) && isset($d['dirty']['name']); });
 
 require_once(INCLUDE_DIR . "class.json.php");
 

include/staff/dynamic-form.inc.php

@@ -123,7 +123,7 @@ $info=Format::htmlchars(($errors && $_POST)?$_POST:$info);
         <tr>
             <td><i class="icon-sort"></i></td>
             <td><input type="text" size="32" name="label-<?php echo $id; ?>"
-                value="<?php echo $f->get('label'); ?>"/>
+                value="<?php echo Format::htmlchars($f->get('label')); ?>"/>
                 <font class="error"><?php
                     if ($ferrors['label']) echo '<br/>'; echo $ferrors['label']; ?>
             </td>
@@ -161,7 +161,8 @@ $info=Format::htmlchars(($errors && $_POST)?$_POST:$info);
             </td>
             <td>
                 <input type="text" size="20" name="name-<?php echo $id; ?>"
-                    value="<?php echo $f->get('name'); ?>" <?php echo $force_name ?>/>
+                    value="<?php echo Format::htmlchars($f->get('name'));
+                    ?>" <?php echo $force_name ?>/>
                 <font class="error"><?php
                     if ($ferrors['name']) echo '<br/>'; echo $ferrors['name'];
                 ?></font>

scp/forms.php

@@ -43,6 +43,8 @@ if($_POST) {
                 }
                 if (in_array($field->get('name'), $names))
                     $field->addError('Field variable name is not unique', 'name');
+                if (preg_match('/[.{}\'"`; ]/u', $field->get('name')))
+                    $field->addError('Invalid character in variable name. Please use letters and numbers only.', 'name');
                 if ($field->get('name'))
                     $names[] = $field->get('name');
                 if ($field->isValid())