Issue #2496565: Added placeholders to the get_select_option_where() method for security and correct quoting (original bug) and ensured that all pertinet where clauses were being added

tripal_views/views/handlers/tripal_views_handler_filter_select_cvterm.inc

@@ -33,15 +33,24 @@ class tripal_views_handler_filter_select_cvterm extends tripal_views_handler_fil
       // we can't assume that tripal admin won't do this) then we only need
       // to make one-hop to the cv table.
       if ($this->table == 'cvterm') {
+
+        $return = $this->get_select_option_where($this->table);
+        $where_clauses = $return['where_clauses'];
+        $arguements = $return['arguements'];
+        $base_where = '';
+        if (!empty($where_clauses)) {
+          $base_where = implode(' AND ', $where_clauses);
+        }
+
         // Using a "Loose Index Scan" to get a list of all the cvs used
         // in the cvterm table (ie: all the cv's with at least one term).
         // See https://wiki.postgresql.org/wiki/Loose_indexscan
         $sql = "
           WITH RECURSIVE t AS (
             SELECT MIN(cv_id) AS col FROM {!table}
-              " . ($where == '' ? '' : "WHERE " . $where) . "
+              " . ($base_where == '' ? '' : "WHERE " . $base_where) . "
             UNION ALL
-            SELECT (SELECT MIN(cv_id) FROM {!table} WHERE cv_id > col " . ($where == '' ? '' : " AND " . $where) . ")
+            SELECT (SELECT MIN(cv_id) FROM {!table} WHERE cv_id > col " . ($base_where == '' ? '' : " AND " . $base_where) . ")
               FROM t WHERE col IS NOT NULL
           )
           SELECT cv_id, name
@@ -53,6 +62,30 @@ class tripal_views_handler_filter_select_cvterm extends tripal_views_handler_fil
       // Otherwise, (most often the case) we need to make two-hops
       // to the cv table through the cvterm table.
       else {
+
+        // There are actually two sets of conditions we care about and of course
+        // they are placed in different places in the query :p.
+        // 1. Restrictions on the cvterm table. This lets users specify: only
+        // show these exact types.
+        $return = $this->get_select_option_where('cvterm');
+        $where_clauses = $return['where_clauses'];
+        $cvterm_args = $return['arguements'];
+        $cvterm_where = '';
+        if (!empty($where_clauses)) {
+          $cvterm_where = implode(' AND ', $where_clauses);
+        }
+        // 2. Restrictions on the filter table Since those affect which types
+        // have been used.
+        $return = $this->get_select_option_where($this->table);
+        $where_clauses = $return['where_clauses'];
+        $base_args = $return['arguements'];
+        $base_where = '';
+        if (!empty($where_clauses)) {
+          $base_where = implode(' AND ', $where_clauses);
+        }
+        // We only supply one set or arguements those so merge the two.
+        $arguements = array_merge($cvterm_args, $base_args);
+
         // Using a "Loose Index Scan" to get a list of all the cvs used
         // in the table the drop-down filter is from.
         // See https://wiki.postgresql.org/wiki/Loose_indexscan
@@ -61,35 +94,29 @@ class tripal_views_handler_filter_select_cvterm extends tripal_views_handler_fil
             SELECT MIN(cvterm.cv_id) AS col
               FROM {!table} filter_table
               LEFT JOIN {cvterm} ON filter_table.!field=cvterm.cvterm_id
-              " . ($where == '' ? '' : "WHERE " . $where) . "
+              " . ($base_where == '' ? '' : "WHERE " . $base_where) . "
             UNION ALL
             SELECT (
                 SELECT MIN(cv_id)
                 FROM {!table} filter_table
                 LEFT JOIN {cvterm} ON filter_table.!field=cvterm.cvterm_id
-                WHERE cv_id > col " . ($where == '' ? '' : " AND " . $where) . "
+                WHERE cv_id > col " . ($base_where == '' ? '' : " AND " . $base_where) . "
               )
               FROM t WHERE col IS NOT NULL
           )
-          SELECT cv_id, name
-            FROM {cv}
-            WHERE cv_id IN (SELECT col FROM t where col IS NOT NULL)
-            ORDER BY cv.name ASC";
+          SELECT cvterm_id, name
+            FROM {cvterm}
+            WHERE cv_id IN (SELECT col FROM t where col IS NOT NULL) " . ($cvterm_where == '' ? '' : " AND " . $cvterm_where) . "
+            ORDER BY cvterm.name ASC";
         $sql = format_string($sql, array('!table' => $this->table, '!field' => $this->field));
       }
-      $resource = chado_query($sql);
+      $resource = chado_query($sql, $arguements);
 
       // Now actually gerenate the select list
       // based on the results from the above query.
       $cvterms = array();
       foreach ($resource as $r) {
-        $results = chado_select_record('cvterm', array('cvterm_id', 'name'), array('cv_id' => $r->cv_id));
-        if (empty($results)) {
-          $results = array();
-        }
-        foreach ($results as $c) {
-          $cvterms[$c->cvterm_id] = $c->name;
-        }
+        $cvterms[$r->cvterm_id] = $r->name;
       }
 
     }
@@ -97,28 +124,50 @@ class tripal_views_handler_filter_select_cvterm extends tripal_views_handler_fil
     // the base table.
     else {
 
-      $where_clauses = $this->get_select_option_where();
-      $where = '';
+      // There are actually two sets of conditions we care about and of course
+      // they are placed in different places in the query :p.
+      // 1. Restrictions on the cvterm table. This lets users specify: only
+      // show these exact types.
+      $return = $this->get_select_option_where('cvterm');
+      $where_clauses = $return['where_clauses'];
+      $cvterm_args = $return['arguements'];
+      $cvterm_where = '';
+      if (!empty($where_clauses)) {
+        $cvterm_where = implode(' AND ', $where_clauses);
+      }
+      // 2. Restrictions on the filter table Since those affect which types
+      // have been used.
+      $return = $this->get_select_option_where($this->table);
+      $where_clauses = $return['where_clauses'];
+      $base_args = $return['arguements'];
+      $base_where = '';
       if (!empty($where_clauses)) {
-        $where = implode(' AND ', $where_clauses);
+        $base_where = implode(' AND ', $where_clauses);
       }
+      // We only supply one set or arguements those so merge the two.
+      $arguements = array_merge($cvterm_args, $base_args);
 
       // Using a "Loose Index Scan" to get a list of all the cvterms used
       // in the base table. See https://wiki.postgresql.org/wiki/Loose_indexscan
       $sql = "
         WITH RECURSIVE t AS (
-          SELECT MIN(!field) AS col FROM {!table} " . ($where == '' ? '' : "WHERE " . $where) . "
+          SELECT MIN(!field) AS col FROM {!table}
+            " . ($base_where == '' ? '' : "WHERE " . $base_where) . "
           UNION ALL
-          SELECT (SELECT MIN(!field) FROM {!table} WHERE !field > col " . ($where == '' ? '' : " AND " . $where) . ")
+          SELECT (
+            SELECT MIN(!field)
+            FROM {!table}
+            WHERE !field > col " . ($base_where == '' ? '' : " AND " . $base_where) . "
+          )
           FROM t WHERE col IS NOT NULL
         )
         SELECT cvterm_id, name
           FROM {cvterm}
-          WHERE cvterm_id IN (SELECT col FROM t where col IS NOT NULL)
+          WHERE cvterm_id IN (SELECT col FROM t where col IS NOT NULL) " . ($cvterm_where == '' ? '' : " AND " . $cvterm_where) . "
           ORDER BY cvterm.name ASC";
       $sql = format_string($sql, array('!table' => $this->table, '!field' => $this->field));
 
-      $resource = chado_query($sql);
+      $resource = chado_query($sql, $arguements);
       $cvterms = array();
 
       // Add an "- Any - " option to allow a type to not be set by default.
@@ -143,9 +192,10 @@ class tripal_views_handler_filter_select_cvterm extends tripal_views_handler_fil
    * @return
    *   An array of full qualified where clauses (ie: table.myfield = 'fred')
    */
-  function get_select_option_where() {
-    $where = array();
-
+  function get_select_option_where($table = NULL, $generic_placeholder = TRUE) {
+    return parent::get_select_option_where($table, $generic_placeholder);
+  }
+/*
     // build a where clause that will filter the list in the drop box
     // using fields that are not exposed and that are for the table
     // from whcih the values in the drop box will be slected and
@@ -182,7 +232,7 @@ class tripal_views_handler_filter_select_cvterm extends tripal_views_handler_fil
 
     return $where;
   }
-
+*/
   /**
    * {@inheritdoc}
    */

tripal_views/views/handlers/tripal_views_handler_filter_select_id.inc

@@ -33,6 +33,8 @@ class tripal_views_handler_filter_select_id extends tripal_views_handler_filter_
    *   An array of options where the key is the value of this field in the database
    */
   function get_select_options() {
+
+    // @TODO: Make name field configurable.
     $name_field = 'common_name';
 
     // First check that this table has a name field.
@@ -45,10 +47,16 @@ class tripal_views_handler_filter_select_id extends tripal_views_handler_filter_
     // the table referenced by the foreign key constraint.
     if (isset($this->options['show_all']) AND $this->options['show_all'] == TRUE) {
 
+      // We still want to use any hidden fitlers on the parent table
+      // but the arguements will need to be field names rather than
+      // generic placeholders so we need to tell get_select_option_where() that.
+      $return = $this->get_select_option_where($this->parent_table, FALSE);
+      $args = $return['arguements'];
+
       // Simply grab all the values from the table referenced by
       // the foreign key constraint. Since we use the id as the key of
       // the options there is no need to use DISTRINCT in the query.
-      $resource = chado_select_record($this->parent_table, array($this->field, $name_field), array());
+      $resource = chado_select_record($this->parent_table, array($this->field, $name_field), $args);
       $options = array();
       foreach ($resource as $r) {
         $options[$r->{$this->field}] = $r->{$name_field};
@@ -57,12 +65,15 @@ class tripal_views_handler_filter_select_id extends tripal_views_handler_filter_
     // Otherwise, only show those that are actually used in the base table.
     else {
 
-      $where_clauses = $this->get_select_option_where();
+      $return = $this->get_select_option_where($this->parent_table);
+      $where_clauses = $return['where_clauses'];
+      $arguements = $return['arguements'];
       $where = '';
       if (!empty($where_clauses)) {
-        $where = ' AND ' . implode(' AND ', $where_clauses);
+        $where = implode(' AND ', $where_clauses);
       }
 
+
       // Using a "Loose Index Scan" to get a list of all the unique values for
       // the name in the table referenced by the foreign key constraint.
       // See https://wiki.postgresql.org/wiki/Loose_indexscan
@@ -88,11 +99,10 @@ class tripal_views_handler_filter_select_id extends tripal_views_handler_filter_
         '!filter_table' => $this->table,
         '!foreign_table' => $this->parent_table,
         '!id_field' => $this->field,
-        // @TODO: Make name field configurable.
         '!name_field' => $name_field
       ));
 
-      $resource = chado_query($sql);
+      $resource = chado_query($sql, $arguements);
       $options = array();
 
       if ($this->options['select_optional']) {
@@ -114,10 +124,8 @@ class tripal_views_handler_filter_select_id extends tripal_views_handler_filter_
    * @return
    *   An array of full qualified where clauses (ie: table.myfield = 'fred')
    */
-  function get_select_option_where() {
-    $where = parent::get_select_option_where();
-
-    return $where;
+  function get_select_option_where($table = NULL, $generic_placeholder = TRUE) {
+    return parent::get_select_option_where($table, $generic_placeholder);
   }
 
   /**

tripal_views/views/handlers/tripal_views_handler_filter_select_string.inc

@@ -86,7 +86,9 @@ class tripal_views_handler_filter_select_string extends views_handler_filter_str
    */
   function get_select_options() {
 
-    $where_clauses = $this->get_select_option_where();
+    $return = $this->get_select_option_where();
+    $where_clauses = $return['where_clauses'];
+    $arguements = $return['arguements'];
     $where = '';
     if (!empty($where_clauses)) {
       $where = ' WHERE ' . implode(' AND ', $where_clauses);
@@ -94,7 +96,7 @@ class tripal_views_handler_filter_select_string extends views_handler_filter_str
 
     // get the values from the table
     $sql = 'SELECT ' . $this->real_field . ' FROM {' . $this->table . '} ' . $where . ' ORDER BY ' . $this->field . ' ASC';
-    $results = chado_query($sql);
+    $results = chado_query($sql, $arguements);
 
     // Build the select box options
     $max_length = (isset($this->options['max_length'])) ? $this->options['max_length'] : 40;
@@ -123,8 +125,11 @@ class tripal_views_handler_filter_select_string extends views_handler_filter_str
    * @return
    *   An array of full qualified where clauses (ie: table.myfield = 'fred')
    */
-  function get_select_option_where() {
+  function get_select_option_where($table = NULL, $generic_placeholder = TRUE) {
     $where = array();
+    $values = array();
+
+    $table = (is_null($table)) ? $this->table : $table;
 
     // build a where clause that will filter the list in the drop box
     // using fields that are not exposed and that are for the table
@@ -133,32 +138,47 @@ class tripal_views_handler_filter_select_string extends views_handler_filter_str
     // available to the user to edit--they're fixed.
     $where = array();
     $filters = (is_array($this->view->filter)) ? $this->view->filter : array();
+    $placeholder_prefix = 'arg';
+    $i = 0;
     foreach ($filters as $filter_name => $details) {
       // we only want to inclue non-exposed filters
       if ($details->options['exposed'] == FALSE) {
+        $i++;
 
         $value = $details->value;
         if (is_array($details->value) AND isset($details->value['value'])) {
          $value = $details->value['value'];
         }
 
-        if (is_array($value)) {
-          // we only want to filter on the table we're getting the list from
-          if (strcmp($details->table, $this->table)==0 AND !empty($value)) {
-            $where[] = "$details->field IN (" . implode(', ', $value) . ')';
-          }
-
+        // Generate the current placeholder.
+        if ($generic_placeholder) {
+          $placeholder = ':'.$placeholder_prefix.$i;
         }
         else {
-          // we only want to filter on the table we're getting the list from
-          if (strcmp($details->table, $this->table)==0 AND !empty($value)) {
-            $where[] = "$details->field $details->operator " . $value;
+          $placeholder = $details->real_field;
+        }
+
+        // we only want to filter on the table we're getting the list from
+        if (strcmp($details->table, $table)==0 AND !empty($value)) {
+
+          // If the value is an array then use IN instead of the choosen operator.
+          if (is_array($value)) {
+              $where[] = "$details->field IN ($placeholder)";
+              $values[$placeholder] = $value;
+          }
+          // Otherwise, just use the operator choosen by the admin.
+          else {
+            $where[] = "$details->field $details->operator $placeholder";
+            $values[$placeholder] = $value;
           }
         }
       }
     }
 
-    return $where;
+    return array(
+      'where_clauses' => $where,
+      'arguements' => $values
+    );
   }
 
   /**