On Jun 10, 2012, at 12:43 PM, A.M. wrote:

> 
> On Jun 9, 2012, at 10:41 PM, Michael Bayer wrote:
> 
>> it looks great.
>> 
>> This is in the queue as http://www.sqlalchemy.org/trac/ticket/2502.
> 
> 
> 1765              text += " MATCH %s" % constraint.match 
> 
> SQL injection? Shouldn't the argument be one of three constants?

This is not a SQL injection vector, for several reasons:

1. its a SQL keyword, not a literal value.  Most database client APIs only 
consider literal values, not SQL keywords or schema identifiers, as candidates 
for bound values.  While some DBAPIs, notably mysql-python, don't make use of 
real bound parameters and instead use Python string interpolation, technically 
they're doing the wrong thing.   This ties into typical prepared statement 
mechanics which allow the database to separate the lexical aspect of a SQL 
statement from the data values.

2. not only is it a SQL keyword, its a keyword that's only used at DDL time.   
SQL injection IMO is not an issue for DDL - no sane application should ever be 
producing DDL at application run time, as the schema is part of the 
application's structure.

3. SQLAlchemy accepts textual SQL statements in many ways.    The developer is 
not absolved from basic practices, just like putting user input into eval() 
should never be done, nor should user input be placed into the lexical portions 
of SQL statements.   Bound values have special meaning within a statement which 
is where library-level interaction is warranted.

In SQLAlchemy, we use literal strings for these values, such as for "ON 
DELETE", "ON UPDATE", "WITH LOCKMODE", SQL hints, etc., so that the full array 
of platform specific values, including new ones that come out in newer 
releases, can be used without SQLAlchemy requiring modification.

All of that said, all these SQL keywords can be tested against a basic sanity 
check, such as one that tests for alphanumeric characters only, so I wouldn't 
oppose a standard "sanity" checker for all the places we accept SQL keywords.

> 
> I suspect there needs to be some specific per-database-driver logic to handle 
> unimplemented cases. PostgreSQL, for example, doesn't support MATCH PARTIAL ( 
> http://www.postgresql.org/docs/9.1/static/sql-createtable.html ) and MySQL, 
> naturally, completely ignores the syntax and triggers other clauses to be 
> ignored:

SQLAlchemy's current technique for this common situation is to use conditional 
DDL techniques:

http://docs.sqlalchemy.org/en/rel_0_7/core/schema.html#sqlalchemy.schema.DDLElement.execute_if
http://docs.sqlalchemy.org/en/rel_0_7/core/schema.html#metadata-ddl


-- 
You received this message because you are subscribed to the Google Groups 
"sqlalchemy" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/sqlalchemy?hl=en.

Reply via email to