Doctrine 1
  1. Doctrine 1
  2. DC-944

Precedence problem in SQL generation allows bypass of pending joins

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.2.3
    • Fix Version/s: None
    • Component/s: Query
    • Labels:
      None
    • Environment:
      PHP 5.2, 5.3

      Description

      'Pending join conditions' are used by listeners to inject extra SQL conditions into a query. They are often used to add basic constraints on every query. An example is the bundled SoftDelete template. Its listener adds extra constraints such as s.deleted_at IS NULL to a query, to make sure that deleted rows are never retrieved on a query.

      However, in the emitted SQL, Doctrine_Query does not use parentheses to group normal SQL conditions together. The pending join condition is simply added to the string without encapsulating existing expressions. This makes it possible to bypass the pending join conditions entirely by using the OR operator.

      Example

      For instance, the following query exhibits this problem:

      $query = Doctrine_Query::create()
      ->from("SoftDeleteTest")
      ->where("name=?", "faulty")
      ->orWhere("name=?", "faulty");

      This query emits the following SQL:

      SELECT s.name AS s_name, s.deleted_at AS s_deleted_at FROM soft_delete_test s WHERE (s.name = 'faulty' OR s.name = 'faulty' AND (s.deleted_at IS NULL))

      which returns also a deleted row.

      Expected behavior

      One would expect the pending join conditions always to hold, and to have precedence over regularly added SQL conditions. This could be accomplished in the most simple fashion by:

      SELECT s.name AS s_name, s.deleted_at AS s_deleted_at FROM soft_delete_test s WHERE ( ( s.name = 'faulty' OR s.name = 'faulty' ) AND (s.deleted_at IS NULL));

      As the existing expressions are now encapsulated by parentheses, it is no longer possible to bypass the pending join conditions injected by the query listener.

      Full test case details:

      init.sql

      create database softdelete;
      grant all privileges on softdelete.* to softdelete@localhost identified by 'uahwqeruwer';
      
      use softdelete;
      CREATE TABLE soft_delete_test (name VARCHAR(255), 
          deleted_at DATETIME DEFAULT NULL, 
          PRIMARY KEY(name)) ENGINE = INNODB;
      
      insert into soft_delete_test values ('fine', null);
      insert into soft_delete_test values ('faulty', now());
      

      run.php

      <?php
      
      require "./1.2.3/lib/Doctrine.php";
      
      spl_autoload_register(array('Doctrine', 'autoload'));
      
      require "SoftDeleteTest.php";
      
      $conn = Doctrine_Manager::connection("mysql://softdelete:uahwqeruwer@localhost/softdelete");
      $conn->setAttribute(Doctrine::ATTR_USE_DQL_CALLBACKS, true);
      
      $query = Doctrine_Query::create()
          ->from("SoftDeleteTest")
          ->where("name=?", "faulty")
          ->orWhere("name=?", "faulty");
      
      $found = $query->execute();
      foreach ($found as $f) {
          echo "ERROR! Found a deleted row: $f->name\n";
      }
      echo "Done.\n";
      

      SoftDeleteTest.php (copied from Doctrine manual)

      <?php
      
      class SoftDeleteTest extends Doctrine_Record
      {
          public function setTableDefinition()
          {
              $this->hasColumn('name', 'string', null, array(
                      'primary' => true
                  )
              );
          }
      
          public function setUp()
          {
              $this->actAs('SoftDelete');
          }
      }
      

        Activity

        Hide
        Walter Hop added a comment -

        Final formatting fixes.

        Show
        Walter Hop added a comment - Final formatting fixes.
        Hide
        Walter Hop added a comment -

        Fixing quote formatting

        Show
        Walter Hop added a comment - Fixing quote formatting

          People

          • Assignee:
            Guilherme Blanco
            Reporter:
            Walter Hop
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: