metainfo: Escaping (#4880)

redaxo · Nov 4, 2021 · d80d797 · d80d797
1 parent fd024a8
commit d80d797
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 44 deletions.
diff --git a/.tools/psalm/baseline.xml b/.tools/psalm/baseline.xml
@@ -1796,7 +1796,7 @@
     <MixedOperand occurrences="3">
       <code>$postValue</code>
       <code>$typeLabel</code>
-      <code>$typeLabel</code>
+      <code>rex_escape($typeLabel)</code>
     </MixedOperand>
     <MixedReturnStatement occurrences="1">
       <code>$saveValue</code>
@@ -1833,11 +1833,10 @@
       <code>$warning[]</code>
       <code>$warning[]</code>
     </MixedArrayAssignment>
-    <MixedAssignment occurrences="5">
+    <MixedAssignment occurrences="4">
       <code>$catId</code>
       <code>$name</code>
       <code>$params['activeItem']</code>
-      <code>$params['id']</code>
       <code>$warning</code>
     </MixedAssignment>
     <MixedInferredReturnType occurrences="1">
@@ -2069,25 +2068,26 @@
     </PossiblyNullOperand>
   </file>
   <file src="redaxo/src/addons/metainfo/lib/table_manager.php">
-    <MixedArgument occurrences="4">
+    <MixedArgument occurrences="12">
       <code>$default</code>
       <code>$default</code>
-      <code>$qry</code>
-      <code>$this-&gt;getTableName()</code>
-    </MixedArgument>
-    <MixedAssignment occurrences="1">
-      <code>$this-&gt;DBID</code>
-    </MixedAssignment>
-    <MixedOperand occurrences="11">
-      <code>$length</code>
-      <code>$length</code>
       <code>$name</code>
       <code>$name</code>
       <code>$name</code>
       <code>$oldname</code>
       <code>$this-&gt;getTableName()</code>
       <code>$this-&gt;getTableName()</code>
       <code>$this-&gt;getTableName()</code>
+      <code>$this-&gt;getTableName()</code>
+      <code>$type</code>
+      <code>$type</code>
+    </MixedArgument>
+    <MixedAssignment occurrences="1">
+      <code>$this-&gt;DBID</code>
+    </MixedAssignment>
+    <MixedOperand occurrences="4">
+      <code>$type</code>
+      <code>$type</code>
       <code>$type</code>
       <code>$type</code>
     </MixedOperand>

diff --git a/redaxo/src/addons/metainfo/functions/function_metainfo.php b/redaxo/src/addons/metainfo/functions/function_metainfo.php
@@ -120,9 +120,9 @@ function rex_metainfo_add_field($title, $name, $priority, $attributes, $type, $d
     $sql->insert();
 
     // replace LIKE wildcards
-    $prefix = str_replace(['_', '%'], ['\_', '\%'], $prefix);
+    $prefix = $sql->escape($sql->escapeLikeWildcards($prefix).'%');
 
-    rex_sql_util::organizePriorities(rex::getTablePrefix() . 'metainfo_field', 'priority', 'name LIKE "' . $prefix . '%"', 'priority, updatedate');
+    rex_sql_util::organizePriorities(rex::getTablePrefix() . 'metainfo_field', 'priority', 'name LIKE ' . $prefix, 'priority, updatedate');
 
     $tableManager = new rex_metainfo_table_manager($metaTable);
     return $tableManager->addColumn($name, $fieldDbType, $fieldDbLength, $default);

diff --git a/redaxo/src/addons/metainfo/lib/handler/handler.php b/redaxo/src/addons/metainfo/lib/handler/handler.php
@@ -181,7 +181,7 @@ public function renderMetaFields(rex_sql $sqlFields, array $epParams)
                             $currentId .= '-'.rex_escape((string) preg_replace('/[^a-zA-Z0-9_-]/', '_', (string) $key));
                             $e['label'] = '<label for="' . $currentId . '">' . rex_escape($value) . '</label>';
                         }
-                        $e['field'] = '<input type="' . $typeLabel . '" name="' . $name . '" value="' . rex_escape($key) . '" id="' . $currentId . '" ' . $attrStr . $selected . ' />';
+                        $e['field'] = '<input type="' . rex_escape($typeLabel) . '" name="' . rex_escape($name) . '" value="' . rex_escape($key) . '" id="' . $currentId . '" ' . $attrStr . $selected . ' />';
                         $formElements[] = $e;
                     }
 

diff --git a/redaxo/src/addons/metainfo/lib/handler/media_handler.php b/redaxo/src/addons/metainfo/lib/handler/media_handler.php
@@ -190,7 +190,7 @@ public function extendForm(rex_extension_point $ep)
             $qry = 'SELECT id FROM ' . rex::getTablePrefix() . 'media WHERE filename="' . $params['filename'] . '"';
             $sql->setQuery($qry);
             if (1 == $sql->getRows()) {
-                $params['id'] = $sql->getValue('id');
+                $params['id'] = (int) $sql->getValue('id');
             } else {
                 throw new rex_exception('Error occured during file upload!');
             }

diff --git a/redaxo/src/addons/metainfo/lib/table_manager.php b/redaxo/src/addons/metainfo/lib/table_manager.php
@@ -51,24 +51,32 @@ public function getTableName()
      */
     public function addColumn($name, $type, $length, $default = null, $nullable = true)
     {
-        $qry = 'ALTER TABLE `' . $this->getTableName() . '` ADD ';
-        $qry .= '`' . $name . '` ' . $type;
+        $sql = rex_sql::factory($this->DBID);
+
+        $qry = 'ALTER TABLE ' . $sql->escapeIdentifier($this->getTableName()) . ' ADD ';
+        $qry .= $sql->escapeIdentifier($name);
+
+        if (!ctype_alpha($type)) {
+            throw new InvalidArgumentException('Invalid column type "'.$type.'"');
+        }
+        /** @psalm-taint-escape sql */
+        $qry .= ' ' . $type;
 
         if (0 != $length) {
-            $qry .= '(' . $length . ')';
+            $qry .= '(' . (int) $length . ')';
         }
 
         // `text` columns in mysql can not have default values
         if ('text' !== $type && null !== $default) {
-            $qry .= ' DEFAULT \'' . str_replace("'", "\'", $default) . '\'';
+            $qry .= ' DEFAULT ' . $sql->escape($default);
         }
 
         if (true !== $nullable) {
             $qry .= ' NOT NULL';
         }
 
         try {
-            $this->setQuery($qry);
+            $sql->setQuery($qry);
             return true;
         } catch (rex_sql_exception $e) {
             return false;
@@ -80,24 +88,32 @@ public function addColumn($name, $type, $length, $default = null, $nullable = tr
      */
     public function editColumn($oldname, $name, $type, $length, $default = null, $nullable = true)
     {
-        $qry = 'ALTER TABLE `' . $this->getTableName() . '` CHANGE ';
-        $qry .= '`' . $oldname . '` `' . $name . '` ' . $type;
+        $sql = rex_sql::factory($this->DBID);
+
+        $qry = 'ALTER TABLE ' . $sql->escapeIdentifier($this->getTableName()) . ' CHANGE ';
+        $qry .= $sql->escapeIdentifier($oldname) . ' ' . $sql->escapeIdentifier($name);
+
+        if (!ctype_alpha($type)) {
+            throw new InvalidArgumentException('Invalid column type "'.$type.'"');
+        }
+        /** @psalm-taint-escape sql */
+        $qry .= ' ' . $type;
 
         if (0 != $length) {
-            $qry .= '(' . $length . ')';
+            $qry .= '(' . (int) $length . ')';
         }
 
         // `text` columns in mysql can not have default values
         if ('text' !== $type && null !== $default) {
-            $qry .= ' DEFAULT \'' . str_replace("'", "\'", $default) . '\'';
+            $qry .= ' DEFAULT ' . $sql->escape($default);
         }
 
         if (true !== $nullable) {
             $qry .= ' NOT NULL';
         }
 
         try {
-            $this->setQuery($qry);
+            $sql->setQuery($qry);
             return true;
         } catch (rex_sql_exception $e) {
             return false;
@@ -109,11 +125,13 @@ public function editColumn($oldname, $name, $type, $length, $default = null, $nu
      */
     public function deleteColumn($name)
     {
-        $qry = 'ALTER TABLE `' . $this->getTableName() . '` DROP ';
-        $qry .= '`' . $name . '`';
+        $sql = rex_sql::factory($this->DBID);
+
+        $qry = 'ALTER TABLE ' . $sql->escapeIdentifier($this->getTableName()) . ' DROP ';
+        $qry .= $sql->escapeIdentifier($name);
 
         try {
-            $this->setQuery($qry);
+            $sql->setQuery($qry);
             return true;
         } catch (rex_sql_exception $e) {
             return false;
@@ -134,18 +152,4 @@ public function hasColumn($name)
         }
         return false;
     }
-
-    /**
-     * @return bool
-     */
-    protected function setQuery($qry)
-    {
-        try {
-            $sql = rex_sql::factory($this->DBID);
-            $sql->setQuery($qry);
-            return true;
-        } catch (rex_sql_exception $e) {
-            return false;
-        }
-    }
 }