From: Simonov Denis <sim-mail@list.ru>
Date: Wed, 18 Dec 2019 19:42:07 +0000 (+0100)
Subject: Fix #64937: Firebird PDO preprocessing sql
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=17a789e27c31ca13ba4bab6fcfc265d2dd0589a2;p=php

Fix #64937: Firebird PDO preprocessing sql

This patch fixes some problems with preprocessing SQL queries.

* The new algorithm takes into account single-line and multi-line
  comments and ignores the ":" and "?" Parameter markers in them.

* The algorithm allows the EXECUTE BLOCK statement to be processed
  correctly. For this statement, it is necessary to search for
  parameter markers between EXECUTE BLOCK and AS, the rest should be
  left as is.

The SQL preprocessing code has been ported from Firebird to handle
EXECUTE STATEMENT.

Closes GH-4920.
---

diff --git a/NEWS b/NEWS
index 940fb2cbf8..25077eaf54 100644
--- a/NEWS
+++ b/NEWS
@@ -4,9 +4,14 @@ PHP                                                                        NEWS
 
 - CLI:
   . Allow debug server binding to an ephemeral port via `-S localhost:0`. (Sara)
+
 - Core:
   . Fixed bug #80109 (Cannot skip arguments when extended debug is enabled).
     (Nikita)
+
+- PDO_Firebird:
+  . Fixed bug #64937 (Firebird PDO preprocessing sql). (Simonov Denis)
+
 - SPL:
   . SplFixedArray is now IteratorAggregate rather than Iterator. (alexdowad)
 
diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c
index 4eb4f10ee0..c27a9e2ed5 100644
--- a/ext/pdo_firebird/firebird_driver.c
+++ b/ext/pdo_firebird/firebird_driver.c
@@ -32,6 +32,428 @@
 static int firebird_alloc_prepare_stmt(pdo_dbh_t*, const char*, size_t, XSQLDA*, isc_stmt_handle*,
 	HashTable*);
 
+const char CHR_LETTER = 1;
+const char CHR_DIGIT = 2;
+const char CHR_IDENT = 4;
+const char CHR_QUOTE = 8;
+const char CHR_WHITE = 16;
+const char CHR_HEX = 32;
+const char CHR_INTRODUCER = 64;
+
+static const char classes_array[] = {
+	/* 000     */ 0,
+	/* 001     */ 0,
+	/* 002     */ 0,
+	/* 003     */ 0,
+	/* 004     */ 0,
+	/* 005     */ 0,
+	/* 006     */ 0,
+	/* 007     */ 0,
+	/* 008     */ 0,
+	/* 009     */ 16, /* CHR_WHITE */
+	/* 010     */ 16, /* CHR_WHITE */
+	/* 011     */ 0,
+	/* 012     */ 0,
+	/* 013     */ 16, /* CHR_WHITE */
+	/* 014     */ 0,
+	/* 015     */ 0,
+	/* 016     */ 0,
+	/* 017     */ 0,
+	/* 018     */ 0,
+	/* 019     */ 0,
+	/* 020     */ 0,
+	/* 021     */ 0,
+	/* 022     */ 0,
+	/* 023     */ 0,
+	/* 024     */ 0,
+	/* 025     */ 0,
+	/* 026     */ 0,
+	/* 027     */ 0,
+	/* 028     */ 0,
+	/* 029     */ 0,
+	/* 030     */ 0,
+	/* 031     */ 0,
+	/* 032     */ 16, /* CHR_WHITE */
+	/* 033  !  */ 0,
+	/* 034  "  */ 8, /* CHR_QUOTE */
+	/* 035  #  */ 0,
+	/* 036  $  */ 4, /* CHR_IDENT */
+	/* 037  %  */ 0,
+	/* 038  &  */ 0,
+	/* 039  '  */ 8, /* CHR_QUOTE */
+	/* 040  (  */ 0,
+	/* 041  )  */ 0,
+	/* 042  *  */ 0,
+	/* 043  +  */ 0,
+	/* 044  ,  */ 0,
+	/* 045  -  */ 0,
+	/* 046  .  */ 0,
+	/* 047  /  */ 0,
+	/* 048  0  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 049  1  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 050  2  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 051  3  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 052  4  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 053  5  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 054  6  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 055  7  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 056  8  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 057  9  */ 38, /* CHR_DIGIT | CHR_IDENT | CHR_HEX */
+	/* 058  :  */ 0,
+	/* 059  ;  */ 0,
+	/* 060  <  */ 0,
+	/* 061  =  */ 0,
+	/* 062  >  */ 0,
+	/* 063  ?  */ 0,
+	/* 064  @  */ 0,
+	/* 065  A  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 066  B  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 067  C  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 068  D  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 069  E  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 070  F  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 071  G  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 072  H  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 073  I  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 074  J  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 075  K  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 076  L  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 077  M  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 078  N  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 079  O  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 080  P  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 081  Q  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 082  R  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 083  S  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 084  T  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 085  U  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 086  V  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 087  W  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 088  X  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 089  Y  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 090  Z  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 091  [  */ 0,
+	/* 092  \  */ 0,
+	/* 093  ]  */ 0,
+	/* 094  ^  */ 0,
+	/* 095  _  */ 65, /* CHR_IDENT | CHR_INTRODUCER */
+	/* 096  `  */ 0,
+	/* 097  a  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 098  b  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 099  c  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 100  d  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 101  e  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 102  f  */ 37, /* CHR_LETTER | CHR_IDENT | CHR_HEX */
+	/* 103  g  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 104  h  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 105  i  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 106  j  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 107  k  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 108  l  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 109  m  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 110  n  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 111  o  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 112  p  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 113  q  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 114  r  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 115  s  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 116  t  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 117  u  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 118  v  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 119  w  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 120  x  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 121  y  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 122  z  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 123  {  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 124  |  */ 0,
+	/* 125  }  */ 5, /* CHR_LETTER | CHR_IDENT */
+	/* 126  ~  */ 0,
+	/* 127     */ 0
+};
+
+inline char classes(char idx)
+{
+	if (idx > 127) return 0;
+	return classes_array[idx];
+}
+
+typedef enum {
+	ttNone,
+	ttWhite,
+	ttComment,
+	ttBrokenComment,
+	ttString,
+	ttParamMark,
+	ttIdent,
+	ttOther
+} FbTokenType;
+
+static FbTokenType getToken(const char** begin, const char* end)
+{
+	FbTokenType ret = ttNone;
+	const char* p = *begin;
+
+	char c = *p++;
+	switch (c)
+	{
+	case ':':
+	case '?':
+		ret = ttParamMark;
+		break;
+
+	case '\'':
+	case '"':
+		while (p < end)
+		{
+			if (*p++ == c)
+			{
+				ret = ttString;
+				break;
+			}
+		}
+		break;
+
+	case '/':
+		if (p < end && *p == '*')
+		{
+			ret = ttBrokenComment;
+			p++;
+			while (p < end)
+			{
+				if (*p++ == '*' && p < end && *p == '/')
+				{
+					p++;
+					ret = ttComment;
+					break;
+				}
+			}
+		}
+		else {
+			ret = ttOther;
+		}
+		break;
+
+	case '-':
+		if (p < end && *p == '-')
+		{
+			while (++p < end)
+			{
+				if (*p == '\r')
+				{
+					p++;
+					if (p < end && *p == '\n')
+						p++;
+					break;
+				}
+				else if (*p == '\n')
+					break;
+			}
+
+			ret = ttComment;
+		}
+		else
+			ret = ttOther;
+		break;
+
+	default:
+		if (classes(c) & CHR_DIGIT)
+		{
+			while (p < end && (classes(*p) & CHR_DIGIT))
+				p++;
+			ret = ttOther;
+		}
+		else if (classes(c) & CHR_IDENT)
+		{
+			while (p < end && (classes(*p) & CHR_IDENT))
+				p++;
+			ret = ttIdent;
+		}
+		else if (classes(c) & CHR_WHITE)
+		{
+			while (p < end && (classes(*p) & CHR_WHITE))
+				p++;
+			ret = ttWhite;
+		}
+		else
+		{
+			while (p < end && !(classes(*p) & (CHR_DIGIT | CHR_IDENT | CHR_WHITE)) &&
+				(*p != '/') && (*p != '-') && (*p != ':') && (*p != '?') &&
+				(*p != '\'') && (*p != '"'))
+			{
+				p++;
+			}
+			ret = ttOther;
+		}
+	}
+
+	*begin = p;
+	return ret;
+}
+
+int preprocess(const char* sql, int sql_len, char* sql_out, HashTable* named_params)
+{
+	zend_bool passAsIs = 1, execBlock = 0;
+	zend_long pindex = -1;
+	char pname[254], ident[253], ident2[253];
+	unsigned int l;
+	const char* p = sql, * end = sql + sql_len;
+	const char* start = p;
+	FbTokenType tok = getToken(&p, end);
+
+	const char* i = start;
+	while (p < end && (tok == ttComment || tok == ttWhite))
+	{
+		i = p;
+		tok = getToken(&p, end);
+	}
+
+	if (p >= end || tok != ttIdent)
+	{
+		/* Execute statement preprocess SQL error */
+		/* Statement expected */
+		return 0;
+	}
+	/* skip leading comments ?? */
+	start = i;
+	l = p - i;
+	/* check the length of the identifier */
+	/* in Firebird 4.0 it is 63 characters, in previous versions 31 bytes */
+	if (l > 252) {
+		return 0;
+	}
+	strncpy(ident, i, l);
+	ident[l] = '\0';
+	if (!strcasecmp(ident, "EXECUTE"))
+	{
+		/* For EXECUTE PROCEDURE and EXECUTE BLOCK statements, named parameters must be processed. */
+		/* However, in EXECUTE BLOCK this is done in a special way. */
+		const char* i2 = p;
+		tok = getToken(&p, end);
+		while (p < end && (tok == ttComment || tok == ttWhite))
+		{
+			i2 = p;
+			tok = getToken(&p, end);
+		}
+		if (p >= end || tok != ttIdent)
+		{
+			/* Execute statement preprocess SQL error */
+			/* Statement expected */
+			return 0;
+		}
+		l = p - i2;
+		/* check the length of the identifier */
+		/* in Firebird 4.0 it is 63 characters, in previous versions 31 bytes */
+		if (l > 252) {
+			return 0;
+		}
+		strncpy(ident2, i2, l);
+		ident2[l] = '\0';
+		execBlock = !strcasecmp(ident2, "BLOCK");
+		passAsIs = 0;
+	}
+	else
+	{
+		/* Named parameters must be processed in the INSERT, UPDATE, DELETE, MERGE statements. */
+		/* If CTEs are present in the query, they begin with the WITH keyword. */
+		passAsIs = strcasecmp(ident, "INSERT") && strcasecmp(ident, "UPDATE") &&
+			strcasecmp(ident, "DELETE") && strcasecmp(ident, "MERGE") &&
+			strcasecmp(ident, "SELECT") && strcasecmp(ident, "WITH");
+	}
+
+	if (passAsIs)
+	{
+		strcpy(sql_out, sql);
+		return 1;
+	}
+
+	strncat(sql_out, start, p - start);
+
+	while (p < end)
+	{
+		start = p;
+		tok = getToken(&p, end);
+		switch (tok)
+		{
+		case ttParamMark:
+			tok = getToken(&p, end);
+			if (tok == ttIdent /*|| tok == ttString*/)
+			{
+				++pindex;
+				l = p - start;
+				/* check the length of the identifier */
+				/* in Firebird 4.0 it is 63 characters, in previous versions 31 bytes */
+				/* + symbol ":" */
+				if (l > 253) {
+					return 0;
+				}
+				strncpy(pname, start, l);
+				pname[l] = '\0';
+				
+				if (named_params) {
+					zval tmp;
+					ZVAL_LONG(&tmp, pindex);
+					zend_hash_str_update(named_params, pname, l, &tmp);
+				}
+
+				strcat(sql_out, "?");
+			}
+			else
+			{
+				if (strncmp(start, "?", 1)) {
+					/* Execute statement preprocess SQL error */
+					/* Parameter name expected */
+					return 0;
+				}
+				++pindex;
+				strncat(sql_out, start, p - start);
+			}
+			break;
+
+		case ttIdent:
+			if (execBlock)
+			{
+				/* In the EXECUTE BLOCK statement, processing must be */
+				/* carried out up to the keyword AS. */
+				l = p - start;
+				/* check the length of the identifier */
+				/* in Firebird 4.0 it is 63 characters, in previous versions 31 bytes */
+				if (l > 252) {
+					return 0;
+				}
+				strncpy(ident, start, l);
+				ident[l] = '\0';
+				if (!strcasecmp(ident, "AS"))
+				{
+					strncat(sql_out, start, end - start);
+					return 1;
+				}
+			}
+
+		case ttWhite:
+		case ttComment:
+		case ttString:
+		case ttOther:
+			strncat(sql_out, start, p - start);
+			break;
+
+		case ttBrokenComment:
+		{
+			/* Execute statement preprocess SQL error */
+			/* Unclosed comment found near ''@1'' */
+			return 0;
+		}
+		break;
+
+
+		case ttNone:
+			/* Execute statement preprocess SQL error */
+			return 0;
+			break;
+		}
+	}
+	return 1;
+}
+
 /* map driver specific error message to PDO error */
 void _firebird_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, char const *file, zend_long line) /* {{{ */
 {
@@ -352,8 +774,7 @@ static int firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const char *sql, size_t s
 	XSQLDA *out_sqlda, isc_stmt_handle *s, HashTable *named_params)
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
-	char *c, *new_sql, in_quote, in_param, pname[64], *ppname;
-	zend_long l, pindex = -1;
+	char *new_sql;
 
 	/* Firebird allows SQL statements up to 64k, so bail if it doesn't fit */
 	if (sql_len > 65536) {
@@ -379,39 +800,12 @@ static int firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const char *sql, size_t s
 
 	/* in order to support named params, which Firebird itself doesn't,
 	   we need to replace :foo by ?, and store the name we just replaced */
-	new_sql = c = emalloc(sql_len+1);
-
-	for (l = in_quote = in_param = 0; l <= sql_len; ++l) {
-		if ( !(in_quote ^= (sql[l] == '\''))) {
-			if (!in_param) {
-				switch (sql[l]) {
-					case ':':
-						in_param = 1;
-						ppname = pname;
-						*ppname++ = sql[l];
-					case '?':
-						*c++ = '?';
-						++pindex;
-					continue;
-				}
-			} else {
-                                if ((in_param &= ((sql[l] >= 'A' && sql[l] <= 'Z') || (sql[l] >= 'a' && sql[l] <= 'z')
-                                        || (sql[l] >= '0' && sql[l] <= '9') || sql[l] == '_' || sql[l] == '-'))) {
-
-
-					*ppname++ = sql[l];
-					continue;
-				} else {
-					*ppname++ = 0;
-					if (named_params) {
-						zval tmp;
-						ZVAL_LONG(&tmp, pindex);
-						zend_hash_str_update(named_params, pname, (unsigned int)(ppname - pname - 1), &tmp);
-					}
-				}
-			}
-		}
-		*c++ = sql[l];
+	new_sql = emalloc(sql_len+1);
+	new_sql[0] = '\0';
+	if (!preprocess(sql, sql_len, new_sql, named_params)) {
+		strcpy(dbh->error_code, "07000");
+		efree(new_sql);			
+		return 0;
 	}
 
 	/* prepare the statement */
diff --git a/ext/pdo_firebird/tests/execute_block.phpt b/ext/pdo_firebird/tests/execute_block.phpt
new file mode 100644
index 0000000000..fe01acb0a8
--- /dev/null
+++ b/ext/pdo_firebird/tests/execute_block.phpt
@@ -0,0 +1,41 @@
+--TEST--
+PDO_Firebird: support EXECUTE BLOCK
+--SKIPIF--
+<?php require('skipif.inc'); 	
+?>
+--FILE--
+<?php
+	require("testdb.inc");
+
+	$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
+
+	$sql = '
+execute block (a int = :e, b int = :d)
+returns (N int, M int)
+as
+declare z int;
+begin
+  select 10
+  from rdb$database
+  into :z;
+  
+  n = a + b + z;
+  m = z * a;
+  suspend;
+end	
+';
+	$query = $dbh->prepare($sql);
+	$query->execute(['d' => 1, 'e' => 2]);
+	$row = $query->fetch(\PDO::FETCH_OBJ);
+	var_dump($row->N);
+	var_dump($row->M);
+
+	unset($query);
+	unset($dbh);
+	echo "done\n";
+
+?>
+--EXPECT--
+int(13)
+int(20)
+done
diff --git a/ext/pdo_firebird/tests/ignore_parammarks.phpt b/ext/pdo_firebird/tests/ignore_parammarks.phpt
new file mode 100644
index 0000000000..0176449204
--- /dev/null
+++ b/ext/pdo_firebird/tests/ignore_parammarks.phpt
@@ -0,0 +1,42 @@
+--TEST--
+PDO_Firebird: ingnore parameter marks in comments
+--SKIPIF--
+<?php require('skipif.inc'); 	
+?>
+--FILE--
+<?php
+	require("testdb.inc");
+
+	$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
+
+	$sql = '
+select 1 as n
+-- :f
+from rdb$database 
+where 1=:d and 2=:e	
+';
+	$query = $dbh->prepare($sql);
+	$query->execute(['d' => 1, 'e' => 2]);
+	$row = $query->fetch(\PDO::FETCH_OBJ);
+	var_dump($row->N);
+	unset($query);
+
+	$sql = '
+select 1 as n
+from rdb$database 
+where 1=:d /* and :f = 5 */ and 2=:e	
+';
+	$query = $dbh->prepare($sql);
+	$query->execute(['d' => 1, 'e' => 2]);
+	$row = $query->fetch(\PDO::FETCH_OBJ);
+	var_dump($row->N);
+	unset($query);	
+	
+	unset($dbh);
+	echo "done\n";
+
+?>
+--EXPECT--
+int(1)
+int(1)
+done