RE: Dynamic SQL: where do you draw the line?

Thanks Arup..... and thanks to all who replied to this thread.

And if anyone is interested, I found a two-part article on SQL Injection and Oracle
by Pete Finnigan at:

http://www.securityfocus.com/infocus/1644 http://www.securityfocus.com/infocus/1646

-----Original Message-----
Sent: Wednesday, June 25, 2003 5:45 PM
To: Multiple recipients of list ORACLE-L

Thomas

In addition to the responses posted by others, regarding the thrashing of shared pool with zillions of once-used code, there is another serious security hole - SQL Injection. You mentioned this is ging to be your eCommerce application database - and will probably attract the hackers like bees to honey.

In your example, if the parameter p_site_guide is passed a value

X' or 'X'='X

Note there is no single quote before or after the value. The dynamic sql will, in addition to all the other parameters, accept this as a valid where clause and rewrite the query as DELETE FROM some_table WHERE ASSET_GUID IN ( '...') AND OBJECT_ID IN (...) AND SITE_GUID = 'X' OR 'X' = 'X'; what do you think the result is going to be?

Your developers are not going to stop at delete only - they will explore this to SELECTs as well. The seriousness of the problem grows - the injectied code could have

X' UNION ALL SELECT CUST_NAME, CREDIT_CARD_NUM, EXP_DATE... Now in addition to the good old data, the hacker also got some more (and unauthorized) valuable "knowledge".

Try to show them the examples and prove how dangerous the practice of dynamic sql is.

HTH. Arup Nanda

• Original Message ----- To: "Multiple recipients of list ORACLE-L" <[EMAIL PROTECTED]> Sent: Wednesday, June 25, 2003 11:04 AM

> I've been fighting an ongoing war with our ecommerce developers, who are
> inordinately fond of writing
> dynamic SQL code that neglects to incorporate bind variables.
Researching
> AskTom I've
> found and utilized different techniques to force bind variables into these
> dynamic SQL queries,
> including the use of application contexts, object types, etc.
>
> However, I'm wondering if I'm making things worse, essentially providing
> them with band-aids, when I should
> be forcing them to change the way they code.
>
> Consider the sample code below (which is a relatively simple example),
which
> is a generic DELETE statement
> generator.
>
> In this situation, the programmers claim the following code is good
> programming practice, promotes
> ease of maintenance, less buggy, and promotes code reusability (their
> definition of reusability is a bit
> different from mine).
>
> I disagreed with them -- not only is this code not reusable at all, with
the
> parsing overhead consequences,
> it's also harder to debug and tune for performance, due to all the
> permutations that needs to be tested.
> My take was that they be far better off writing a simple static DELETE
> statement for each table.
>
> Their rejoinder -- it's not worth writing lots of redundant code at the
> expense of 'minimal' gains in
> performance. Now, this code *could* be rewritten to use the SYS_CONTEXT
> function on the p_object_id
> and p_site_guid to force a bind variable on those two conditions, but the
IN
> condition with respect to the
> p_asset_guid would be more problematic. However, I don't feel we should
> have to be resorting to such measures
> to get this code to using bind variables.
>
> So, I'm at the point of denying such code to be migrated to production.
I
> recognize that there
> are situations where there is a legitimate need for dynamic SQL, but the
SQL
> has to be written w/o catenating
> literal SQL -- and if it can't -- they need to go back to square one.
>
> Opinions? I'm curious -- do you have policies/standards with respect to
> dynamic SQL?
>
>
>
> CREATE OR REPLACE PROCEDURE test
> (
> p_Asset_Guid IN VARCHAR2,
> p_Object_Id IN VARCHAR2,
> p_Object_Definition IN VARCHAR2,
> p_Site_Guid IN VARCHAR2,
> p_result IN OUT VARCHAR2
>
> ) AS
>
> strTableName VARCHAR2(100);
> strWhere VARCHAR2(100);
> strQuery LONG;
>
> BEGIN
> IF p_Object_Definition = 'PRODUCT'
> THEN
> strTableName := ' TNE.GPD_PRODUCT_ASSET ';
> strWhere := ' MODEL_NO ';
> ELSIF p_Object_Definition = 'CARACTERISTIC'
> OR p_Object_Definition = 'CHARACTERISTIC'
> THEN
> strTableName := ' TNE.GPD_CARACTERISTIC_ASSET ';
> strWhere := ' CARACTERISTIC_ID ';
> ELSIF p_Object_Definition = 'CATEGORY'
> THEN
> strTableName := ' TNE.GPD_CATEGORY_ASSET ';
> strWhere := ' CATEGORY_GUID ';
> ELSIF p_Object_Definition = 'VALUE'
> THEN
> strTableName := ' TNE.GPD_VALUE_ASSET ';
> strWhere := ' VALUE_ID ';
> ELSIF p_Object_Definition = 'PRODUCT_NODE'
> THEN
> strTableName := ' TNE.GPD_PRODUCT_NODE_ASSET ';
> strWhere := ' PRODUCT_NODE_GUID ';
> ELSIF p_Object_Definition = 'CARAC_GROUP'
> THEN
> strTableName := ' TNE.GPD_CARAC_GROUP_ASSET ';
> strWhere := ' CARAC_GROUP_GUID ';
> END IF;
>
> strQuery := ' DELETE FROM ' || strTableName ||
> ' WHERE ASSET_GUID IN ( ''' || REPLACE(p_Asset_Guid,' ',
> ''',''') ||
> ''' ) AND ';
>
> strQuery := strQuery || strWhere || ' = ''' || p_Object_Id || '''';
> strQuery := strQuery || ' AND SITE_GUID = ''' || p_Site_Guid || '''';
>
> execute immediate strQuery;
>
> p_result := '1';
>
> RETURN;
> END;
>
>
>
>
> --------------------------------------------
> Jeffery D Thomas
> DBA
> Thomson Information Services
> Thomson, Inc.
>
> Email: [EMAIL PROTECTED]
>
> Indy DBA Master Documentation available at:
> http://gkmqp.tce.com/tis_dba
> --------------------------------------------
>
>
> --
> Please see the official ORACLE-L FAQ: http://www.orafaq.net
> --
> Author: Thomas Jeff
> INET: [EMAIL PROTECTED]
>
> Fat City Network Services -- 858-538-5051 http://www.fatcity.com
> San Diego, California -- Mailing list and web hosting services
> ---------------------------------------------------------------------
> To REMOVE yourself from this mailing list, send an E-Mail message
> to: [EMAIL PROTECTED] (note EXACT spelling of 'ListGuru') and in
> the message BODY, include a line containing: UNSUB ORACLE-L
> (or the name of mailing list you want to be removed from). You may
> also send the HELP command for other information (like subscribing).
>

-- 
Please see the official ORACLE-L FAQ: http://www.orafaq.net
-- 
Author: Arup Nanda
  INET: [EMAIL PROTECTED]

Fat City Network Services    -- 858-538-5051 http://www.fatcity.com
San Diego, California        -- Mailing list and web hosting services
---------------------------------------------------------------------
To REMOVE yourself from this mailing list, send an E-Mail message
to: [EMAIL PROTECTED] (note EXACT spelling of 'ListGuru') and in
the message BODY, include a line containing: UNSUB ORACLE-L
(or the name of mailing list you want to be removed from).  You may
also send the HELP command for other information (like subscribing).
-- 
Please see the official ORACLE-L FAQ: http://www.orafaq.net
-- 
Author: Thomas Jeff
  INET: [EMAIL PROTECTED]

Fat City Network Services    -- 858-538-5051 http://www.fatcity.com
San Diego, California        -- Mailing list and web hosting services
---------------------------------------------------------------------
To REMOVE yourself from this mailing list, send an E-Mail message
to: [EMAIL PROTECTED] (note EXACT spelling of 'ListGuru') and in
the message BODY, include a line containing: UNSUB ORACLE-L
(or the name of mailing list you want to be removed from).  You may
also send the HELP command for other information (like subscribing).