CODEWITHSUNDEEP
sqloracle-databaseplsql
sunleo
sunleo

Reputation: 10943

Oracle Secure Usage of SQL script

I tried to use secured SQL script using this link but when dynamic binding variables become more and more in number, the code seems to be clumsy.Please check this code and give your idea to reduce the complexity.

Based on the below code, you can see these three variables (ZONE_CODE_STR,ACTIVE_STR and LOGICAL_ENV_STR) as parameters for the query, when value comes I use them or I replace them with an empty string.But the problem is when I want to add one more parameter the IF-ELSE logic becomes too complex.Please help me to find an easy way to achieve if I want to add one more new dynamic variable.

Code Sample:

    -- APPLICATION VARIABLES STARTS HERE
    ZONE_CODE_STR VARCHAR2(100);
    ZONE_CODE_FLAG VARCHAR2(20);
    ACTIVE_STR VARCHAR2(100);
    ACTIVE_FLAG VARCHAR2(20);
    LOGICAL_ENV_STR VARCHAR2(100);
    LOGICAL_ENV_FLAG VARCHAR2(20);
    QUERY_STR VARCHAR2(4000);
    -- APPLICATION VARIABLES END HERE
BEGIN



-- APPLICATION LOGIC STARTS HERE
  ZONE_CODE_STR  := ' AND ZONE_ENV_CODE  IN (: IN_ZONE_ENV_CODE)';
  ZONE_CODE_FLAG := '@@ZONE_CODE@@';
  LOGICAL_ENV_STR  := ' AND LOGICAL_DB_ENVIRONMENT.ZONE_ENV_CODE IN(: IN_ZONE_ENV_CODE)';
  LOGICAL_ENV_FLAG := '@@LOGICAL_ENV@@';
  ACTIVE_STR     := ' AND ACTIVE  =: IN_ACTIVE';
  ACTIVE_FLAG    := '@@ACTIVE@@';
  QUERY_STR  :='SELECT
RES_DETAIL.FRAME_NAME,RES_DETAIL.LISTENER_PORT,
RES_DETAIL.PROPERTY_NAME,RES_DETAIL.PROPERTY_VALUE,RES_DETAIL.IP_ADDRESS,
TEST_APP_ENTRIES.*,
TEST_APP_WEB_SERVER_TYPE.APP_WEB_SERVER_TYPE_NAME,
TEST_OPERATION_TYPE.OPERATION_TYPE_NAME,
TEST_SINGLE_SIGN_ON.SINGLE_SIGN_ON_NAME,TEST_APP_REALM_ENTRIES.WARE_ID,TEST_APP_REALM_ENTRIES.RESOURCE_FILTER,
TEST_APP_REALM_ENTRIES.PROTECTION_STATUS, TEST_APP_REALM_ENTRIES.AUTHENTICATION_TYPE_ID,
TEST_AUTHENTICATION_TYPE.AUTHENTICATION_TYPE_NAME,
TEST_APP_REALM_ENTRIES.ROLE
FROM 
  (
  SELECT * FROM TEST_APP_ENTRIES WHERE APP_EXT_CODE =: IN_APP_EXT_CODE
@@ZONE_CODE@@
@@ACTIVE@@
  )TEST_APP_ENTRIES
  INNER JOIN
  TEST_APP_REALM_ENTRIES
ON
  TEST_APP_ENTRIES.WAE_ID = TEST_APP_REALM_ENTRIES.WAE_ID

INNER JOIN
  TEST_OPERATION_TYPE
ON
  TEST_APP_ENTRIES.OPERATION_TYPE_ID = TEST_OPERATION_TYPE.OPERATION_TYPE_ID
INNER JOIN
  TEST_APP_WEB_SERVER_TYPE
ON
  TEST_APP_ENTRIES.APP_WEB_SERVER_TYPE_ID = TEST_APP_WEB_SERVER_TYPE.APP_WEB_SERVER_TYPE_ID
INNER JOIN
  TEST_SINGLE_SIGN_ON
ON
  TEST_APP_ENTRIES.SINGLE_SIGN_ON_ID = TEST_SINGLE_SIGN_ON.SINGLE_SIGN_ON_ID
INNER JOIN
  TEST_AUTHENTICATION_TYPE
ON
  TEST_APP_REALM_ENTRIES.AUTHENTICATION_TYPE_ID = TEST_AUTHENTICATION_TYPE.AUTHENTICATION_TYPE_ID
RIGHT OUTER JOIN
  (SELECT APP_FRAMES.LOGICAL_DB_ENV_NAME AS LOGICAL_DB_ENV_NAME,APP_FRAME_PROPERTIES.FRAME_NAME AS FRAME_NAME,APP_FRAME_PROPERTIES.APP_EXT_CODE AS APP_EXT_CODE,FRAMES.LISTENER_PORT AS LISTENER_PORT,
  LOGICAL_DB_ENVIRONMENT.ZONE_ENV_CODE AS ZONE_ENV_CODE,
APP_FRAME_PROPERTIES.PROPERTY_NAME AS PROPERTY_NAME,APP_FRAME_PROPERTIES.PROPERTY_VALUE AS PROPERTY_VALUE,NODES.IP_ADDRESS AS IP_ADDRESS
FROM APP_FRAME_PROPERTIES,NODES,FRAMES,APP_FRAMES,LOGICAL_DB_ENVIRONMENT
WHERE APP_FRAME_PROPERTIES.FRAME_NAME = FRAMES.FRAME_NAME
AND APP_FRAME_PROPERTIES.APP_EXT_CODE = APP_FRAMES.APP_EXT_CODE
AND LOGICAL_DB_ENVIRONMENT.APP_EXT_CODE = APP_FRAMES.APP_EXT_CODE
AND LOGICAL_DB_ENVIRONMENT.LOGICAL_DB_ENV_NAME = APP_FRAMES.LOGICAL_DB_ENV_NAME
AND APP_FRAME_PROPERTIES.LOGICAL_DB_ENV_NAME = APP_FRAMES.LOGICAL_DB_ENV_NAME
AND NODES.FRAME_NAME = APP_FRAME_PROPERTIES.FRAME_NAME 
AND APP_FRAMES.FRAME_NAME = APP_FRAME_PROPERTIES.FRAME_NAME
AND APP_FRAMES.APP_EXT_CODE =: IN_APP_EXT_CODE
@@LOGICAL_ENV@@
) RES_DETAIL
ON
  TEST_APP_ENTRIES.APP_EXT_CODE = RES_DETAIL.APP_EXT_CODE
  AND TEST_APP_ENTRIES.ZONE_ENV_CODE = RES_DETAIL.ZONE_ENV_CODE ORDER BY TEST_APP_ENTRIES.WAE_ID,WARE_ID';

        IF(IN_ZONE_ENV_CODE IS NOT NULL AND IN_ZONE_ENV_CODE = 'UAT') THEN
                IN_ZONE_ENV_CODE := 'DEV';
        ELSIF(IN_ZONE_ENV_CODE IS NOT NULL AND IN_ZONE_ENV_CODE = 'PROD') THEN
                IN_ZONE_ENV_CODE := 'UAT';
        END IF;

    IF(IN_ZONE_ENV_CODE IS NULL OR IN_ZONE_ENV_CODE = '' OR IN_ZONE_ENV_CODE = 'ALL') THEN
                QUERY_STR := REPLACE(QUERY_STR,ZONE_CODE_FLAG,'');
                QUERY_STR := REPLACE(QUERY_STR,LOGICAL_ENV_FLAG,'');
                IF(IN_ACTIVE IS NULL OR IN_ACTIVE = '') THEN
                        QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,'');
                        OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_APP_EXT_CODE;
                ELSE
                        QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,ACTIVE_STR);
                        OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_ACTIVE,IN_APP_EXT_CODE;
                END IF;
        ELSIF (IN_ACTIVE IS NULL OR IN_ACTIVE = '') THEN
                        QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,'');
                        QUERY_STR := REPLACE(QUERY_STR,ZONE_CODE_FLAG,ZONE_CODE_STR);
                        QUERY_STR := REPLACE(QUERY_STR,LOGICAL_ENV_FLAG,LOGICAL_ENV_STR);

                        OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_ZONE_ENV_CODE,IN_APP_EXT_CODE,IN_ZONE_ENV_CODE;
        ELSE
                        QUERY_STR := REPLACE(QUERY_STR,ZONE_CODE_FLAG,ZONE_CODE_STR);
                        QUERY_STR := REPLACE(QUERY_STR,LOGICAL_ENV_FLAG,LOGICAL_ENV_STR);
                        QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,ACTIVE_STR);

                        OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_ZONE_ENV_CODE,IN_ACTIVE,IN_APP_EXT_CODE,IN_ZONE_ENV_CODE;   
        END IF;

Upvotes: 0

Views: 57

Answers (2)

Boneist
Boneist

Reputation: 23578

I think you could do what you're after with the following static sql:

select res_detail.frame_name,
       res_detail.listener_port,
       res_detail.property_name,
       res_detail.property_value,
       res_detail.ip_address,
       test_app_entries.*,
       test_app_web_server_type.app_web_server_type_name,
       test_operation_type.operation_type_name,
       test_single_sign_on.single_sign_on_name,
       test_app_realm_entries.ware_id,
       test_app_realm_entries.resource_filter,
       test_app_realm_entries.protection_status,
       test_app_realm_entries.authentication_type_id,
       test_authentication_type.authentication_type_name,
       test_app_realm_entries.role
from   (select *
        from   test_app_entries
        where  app_ext_code = in_app_ext_code
        and    (zone_env_code = in_zone_env_code or in_zone_env_code is null) -- changed
        and    (active = in_active or in_active is null)                      -- changed
       ) test_app_entries
       inner join test_app_realm_entries on test_app_entries.wae_id = test_app_realm_entries.wae_id
       inner join test_operation_type on test_app_entries.operation_type_id = test_operation_type.operation_type_id
       inner join test_app_web_server_type on test_app_entries.app_web_server_type_id = test_app_web_server_type.app_web_server_type_id
       inner join test_single_sign_on on test_app_entries.single_sign_on_id = test_single_sign_on.single_sign_on_id
       inner join test_authentication_type on test_app_realm_entries.authentication_type_id = test_authentication_type.authentication_type_id
       right outer join (select app_frames.logical_db_env_name as logical_db_env_name,
                                app_frame_properties.frame_name as frame_name,
                                app_frame_properties.app_ext_code as app_ext_code,
                                frames.listener_port as listener_port,
                                logical_db_environment.zone_env_code as zone_env_code,
                                app_frame_properties.property_name as property_name,
                                app_frame_properties.property_value as property_value,
                                nodes.ip_address as ip_address
                         from   app_frame_properties,
                                nodes,
                                frames,
                                app_frames,
                                logical_db_environment
                         where  app_frame_properties.frame_name = frames.frame_name
                         and    app_frame_properties.app_ext_code = app_frames.app_ext_code
                         and    logical_db_environment.app_ext_code = app_frames.app_ext_code
                         and    logical_db_environment.logical_db_env_name = app_frames.logical_db_env_name
                         and    app_frame_properties.logical_db_env_name = app_frames.logical_db_env_name
                         and    nodes.frame_name = app_frame_properties.frame_name
                         and    app_frames.frame_name = app_frame_properties.frame_name
                         and    app_frames.app_ext_code = in_app_ext_code
                         and    (logical_db_environment.zone_env_code = in_zone_env_code or in_zone_env_code is null) -- changed
                        ) res_detail on test_app_entries.app_ext_code = res_detail.app_ext_code
                                        and test_app_entries.zone_env_code = res_detail.zone_env_code
order by test_app_entries.wae_id, ware_id;

So where you were previously testing for the parameters being null or not and then building that into the where clauses where appropriate, instead I've replaced them with (parameter = some_col or parameter is null)

You'd have to test with your data to make sure that the performance is still satisfactory.

Additional notes:

  1. I would use short table aliases in place of your full table names; it's more readable that way.
  2. You're using ANSI SQL join syntax for your outer query's joins, but in the res_detail subquery, you're using the old style joins. Switch this to use the ANSI SQL join syntax.
  3. ware_id in the order by is missing it's alias. Which table does it come from?
  4. test_app_entries.* - in production code, it's a bad idea to use * in the outer select; what happens if someone adds a column?

In general, it's better to avoid dynamic sql if at all possible - you won't have issues with security if you aren't concatenating additional text into your sql statement, plus in general it's easier to maintain and debug.

Upvotes: 2

gmiley
gmiley

Reputation: 6604

In your PL SQL above, you are using BIND VARIABLES in some of it, however, you are still creating dynamic SQL using concatenation (in this case using the REPLACE function). The query you supplied is rather large and complicated, so I will use just a simplified selection of it to give you an example.

-- Your stripped-down code:
ZONE_CODE_STR  := ' AND ZONE_ENV_CODE  IN ( :IN_ZONE_ENV_CODE )';
ZONE_CODE_FLAG := '@@ZONE_CODE@@';
ACTIVE_STR     := ' AND ACTIVE  = :IN_ACTIVE ';
ACTIVE_FLAG    := '@@ACTIVE@@';
QUERY_STR  :='SELECT * FROM TEST_APP_ENTRIES WHERE APP_EXT_CODE = :IN_APP_EXT_CODE @@ZONE_CODE@@ @@ACTIVE@@ ';

IF(IN_ZONE_ENV_CODE IS NOT NULL AND IN_ZONE_ENV_CODE = 'UAT') THEN
    IN_ZONE_ENV_CODE := 'DEV';
ELSIF(IN_ZONE_ENV_CODE IS NOT NULL AND IN_ZONE_ENV_CODE = 'PROD') THEN
    IN_ZONE_ENV_CODE := 'UAT';
END IF;

IF(IN_ZONE_ENV_CODE IS NULL OR IN_ZONE_ENV_CODE = '' OR IN_ZONE_ENV_CODE = 'ALL') THEN
    QUERY_STR := REPLACE(QUERY_STR,ZONE_CODE_FLAG,'');
    QUERY_STR := REPLACE(QUERY_STR,LOGICAL_ENV_FLAG,'');
    IF(IN_ACTIVE IS NULL OR IN_ACTIVE = '') THEN
        QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,'');
        OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_APP_EXT_CODE;
    ELSE
        QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,ACTIVE_STR);
        OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_ACTIVE,IN_APP_EXT_CODE;
    END IF;
ELSIF (IN_ACTIVE IS NULL OR IN_ACTIVE = '') THEN
    QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,'');
    QUERY_STR := REPLACE(QUERY_STR,ZONE_CODE_FLAG,ZONE_CODE_STR);
    QUERY_STR := REPLACE(QUERY_STR,LOGICAL_ENV_FLAG,LOGICAL_ENV_STR);
    OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_ZONE_ENV_CODE,IN_APP_EXT_CODE,IN_ZONE_ENV_CODE;
ELSE
    QUERY_STR := REPLACE(QUERY_STR,ZONE_CODE_FLAG,ZONE_CODE_STR);
    QUERY_STR := REPLACE(QUERY_STR,LOGICAL_ENV_FLAG,LOGICAL_ENV_STR);
    QUERY_STR := REPLACE(QUERY_STR,ACTIVE_FLAG,ACTIVE_STR);
    OPEN OUT_RESULT FOR QUERY_STR USING IN_APP_EXT_CODE,IN_ZONE_ENV_CODE,IN_ACTIVE,IN_APP_EXT_CODE,IN_ZONE_ENV_CODE;   
END IF;

What I suggest doing is to refactor your actual SQL query to do away with a lot, if not all, of the extra PL/SQL code related to having the need to rewrite your query dynamically... In the code below I will explain what is going on:

SELECT * FROM TEST_APP_ENTRIES 
WHERE APP_EXT_CODE = IN_APP_EXT_CODE
AND CASE Coalesce(:IN_ZONE_ENV_CODE, 'ALL')  -- See Note 1 below...
        WHEN 'ALL' THEN ZONE_ENV_CODE
        ELSE :IN_ZONE_ENV_CODE 
    END = ZONE_ENV_CODE 
AND Coalesce(:IN_ACTIVE, ACTIVE) = ACTIVE; -- See Note 2 below...

  • Note 1: Instead of checking for the variable to be null and then replacing a place-holder, we can just use the COALESCE function (which returns the first non-null item in the comma-separated list), and compare that in the WHEN portion to the literal value of 'ALL' (which we will evaluate TRUE on the compare if the :IN_ACTIVE variable is either NULL or it actually contains the value of ALL) and we will use ZONE_ENV_CODE, otherwise if that does not evaluate TRUE we go to the ELSE portion of the WHEN and use :IN_ZONE_ENV_CODE instead. One of those two will then compare against ZONE_ENV_CODE. This means that, if :IN_ZONE_ENV_CODE is either NULL or ALL we use ZONE_ENV_CODE = ZONE_ENV_CODE in the where clause, otherwise we use :IN_ZONE_ENV_CODE = ZONE_ENV_CODE instead.

  • Note 2: Is similar, but much simpler... We just use COALESCE to compare ACTIVE = ACTIVE if :IN_ACTIVE is NULL, otherwise :IN_ACTIVE = ACTIVE.

Upvotes: 1

Related Questions