Fixes #921 - Assert input params in dynamic SQL

PhilippSalvisberg · PhilippSalvisberg · commit 938907453472 · 2019-06-01T13:49:18.000+02:00
Used dbms_assert.qualified_sql_name whenever possible.
This is much less invasive than dbms_assert.enquote_name.
Code converting case is left untouched.
diff --git a/source/core/annotations/ut_annotation_manager.pkb b/source/core/annotations/ut_annotation_manager.pkb
@@ -57,7 +57,15 @@ create or replace package body ut_annotation_manager as
     l_objects_view varchar2(200) := ut_metadata.get_objects_view_name();
     l_cursor_text  varchar2(32767);
     l_result       ut_annotation_objs_cache_info;
+    l_object_owner varchar2(250);
+    l_object_type  varchar2(250);
   begin
+    if a_object_owner is not null then
+      l_object_owner := sys.dbms_assert.qualified_sql_name(a_object_owner);
+    end if;
+    if a_object_type is not null then
+      l_object_type := sys.dbms_assert.qualified_sql_name(a_object_type);
+    end if;
     l_cursor_text :=
       q'[select ]'||l_ut_owner||q'[.ut_annotation_obj_cache_info(
                     object_owner => o.owner,
@@ -70,8 +78,8 @@ create or replace package body ut_annotation_manager as
              on o.owner = i.object_owner
             and o.object_name = i.object_name
             and o.object_type = i.object_type
-          where o.owner = ']'||a_object_owner||q'['
-            and o.object_type = ']'||a_object_type||q'['
+          where o.owner = ']'||l_object_owner||q'['
+            and o.object_type = ']'||l_object_type||q'['
             and ]'
         || case
            when a_parse_date is null
diff --git a/source/core/coverage/ut_coverage.pkb b/source/core/coverage/ut_coverage.pkb
@@ -107,12 +107,18 @@ create or replace package body ut_coverage is
     l_cursor        sys_refcursor;
     l_skip_objects  ut_object_names;
     l_sql           varchar2(32767);
+    l_valid_pattern varchar2(250) := '^\s*select.+$';
   begin
     if not is_develop_mode() then
       --skip all the utplsql framework objects and all the unit test packages that could potentially be reported by coverage.
       l_skip_objects := ut_utils.get_utplsql_objects_list() multiset union all coalesce(a_coverage_options.exclude_objects, ut_object_names());
     end if;
-    l_sql := a_sql;
+    if regexp_like(a_sql, l_valid_pattern, 'mi') then
+      -- pseudo assert for PL/SQL Cop
+      l_sql := sys.dbms_assert.noop(a_sql); 
+    else
+      raise_application_error(-20542, 'Possible SQL injection detected. a_sql parameter does not match valid pattern "' || l_valid_pattern || '".');
+    end if;
     if a_coverage_options.file_mappings is not empty then
       open l_cursor for l_sql using a_coverage_options.file_mappings, l_skip_objects;
     elsif a_coverage_options.include_objects is not empty then
diff --git a/source/core/types/ut_suite_item.tpb b/source/core/types/ut_suite_item.tpb
@@ -63,9 +63,11 @@ create or replace type body ut_suite_item as
     ex_savepoint_not_exists exception;
     l_transaction_invalidators clob;
     pragma exception_init(ex_savepoint_not_exists, -1086);
+    l_savepoint varchar2(250);
   begin
     if get_rollback_type() = ut_utils.gc_rollback_auto and a_savepoint is not null then
-      execute immediate 'rollback to ' || a_savepoint;
+      l_savepoint := sys.dbms_assert.qualified_sql_name(a_savepoint);
+      execute immediate 'rollback to ' || l_savepoint;
     end if;
   exception
     when ex_savepoint_not_exists then
diff --git a/source/core/ut_metadata.pkb b/source/core/ut_metadata.pkb
@@ -306,15 +306,17 @@ create or replace package body ut_metadata as
   function is_anytype_null(a_value in anydata, a_compound_type in varchar2) return number is
     l_result integer := 0;
     l_anydata_sql varchar2(4000);
+    l_compound_type varchar2(250);
   begin
-     if a_value is not null then
-     l_anydata_sql := '
+    if a_value is not null then
+      l_compound_type := sys.dbms_assert.qualified_sql_name(a_compound_type);
+      l_anydata_sql := '
         declare
           l_data '||get_anydata_typename(a_value)||';
           l_value anydata := :a_value;
           l_status integer;
         begin
-          l_status := l_value.get'||a_compound_type||'(l_data);
+          l_status := l_value.get'||l_compound_type||'(l_data);
           :l_data_is_null := case when l_data is null then 1 else 0 end; 
         end;';
         execute immediate l_anydata_sql using in a_value, out l_result; 
diff --git a/source/core/ut_suite_manager.pkb b/source/core/ut_suite_manager.pkb
@@ -335,7 +335,9 @@ create or replace package body ut_suite_manager is
     l_objects_view varchar2(200) := ut_metadata.get_objects_view_name();
     l_cursor_text  varchar2(32767);
     l_result       ut_varchar2_rows;
+    l_object_owner varchar2(250);
   begin
+    l_object_owner := sys.dbms_assert.qualified_sql_name(a_object_owner);
     l_cursor_text :=
       q'[select i.object_name
          from ]'||l_ut_owner||q'[.ut_suite_cache_package i
@@ -345,9 +347,9 @@ create or replace package body ut_suite_manager is
                where o.owner = i.object_owner
                  and o.object_name = i.object_name
                  and o.object_type = 'PACKAGE'
-                 and o.owner = ']'||a_object_owner||q'['
+                 and o.owner = ']'||l_object_owner||q'['
               )
-          and i.object_owner = ']'||a_object_owner||q'[']';
+          and i.object_owner = ']'||l_object_owner||q'[']';
     open l_rows for l_cursor_text;
     fetch l_rows bulk collect into l_result limit 1000000;
     close l_rows;
@@ -362,19 +364,33 @@ create or replace package body ut_suite_manager is
     a_skip_all_objects boolean  := false,
     a_random_seed      positive
   ) return t_cached_suites_cursor is
-    l_path     varchar2( 4000 );
-    l_result   sys_refcursor;
-    l_ut_owner varchar2(250) := ut_utils.ut_owner;
+    l_path           varchar2(4000);
+    l_result         sys_refcursor;
+    l_ut_owner       varchar2(250) := ut_utils.ut_owner;
+    l_object_owner   varchar2(250);
+    l_object_name    varchar2(250);
+    l_procedure_name varchar2(250);
   begin
+    if a_object_owner is not null then
+      l_object_owner := sys.dbms_assert.qualified_sql_name(a_object_owner);
+    end if;
+    if a_object_name is not null then
+      l_object_name := sys.dbms_assert.qualified_sql_name(a_object_name);
+    end if;
+    if a_procedure_name is not null then
+      l_procedure_name := sys.dbms_assert.qualified_sql_name(a_procedure_name);
+    end if;
     if a_path is null and a_object_name is not null then
       execute immediate 'select min(path)
       from '||l_ut_owner||q'[.ut_suite_cache
      where object_owner = :a_object_owner
            and object_name = :a_object_name
            and name = nvl(:a_procedure_name, name)]'
-      into l_path using upper(a_object_owner), upper(a_object_name), upper(a_procedure_name);
+      into l_path using upper(l_object_owner), upper(l_object_name), upper(l_procedure_name);
     else
-      l_path := lower( a_path );
+      if a_path is not null then
+        l_path := lower(sys.dbms_assert.qualified_sql_name(a_path));
+      end if;
     end if;
 
     open l_result for
@@ -717,29 +733,37 @@ create or replace package body ut_suite_manager is
     a_owner_name     varchar2, 
     a_package_name   varchar2 := null
   ) return sys_refcursor is
-    l_result      sys_refcursor;
-    l_ut_owner    varchar2(250) := ut_utils.ut_owner;
+    l_result       sys_refcursor;
+    l_ut_owner     varchar2(250) := ut_utils.ut_owner;
+    l_owner_name   varchar2(250);
+    l_package_name varchar2(250);
   begin
+    if a_owner_name is not null then
+      l_owner_name := sys.dbms_assert.qualified_sql_name(a_owner_name);
+    end if;
+    if a_package_name is not null then
+      l_package_name := sys.dbms_assert.qualified_sql_name(a_package_name);
+    end if;
 
-    refresh_cache(a_owner_name);
+    refresh_cache(l_owner_name);
     
     open l_result for
     q'[with
       suite_items as (
         select /*+ cardinality(c 100) */ c.*
           from ]'||l_ut_owner||q'[.ut_suite_cache c
-         where 1 = 1 ]'||case when can_skip_all_objects_scan(a_owner_name) then q'[
+         where 1 = 1 ]'||case when can_skip_all_objects_scan(l_owner_name) then q'[
                and exists
                    ( select 1
                        from all_objects a
                       where a.object_name = c.object_name
-                        and a.owner       = ']'||upper(a_owner_name)||q'['
+                        and a.owner       = ']'||upper(l_owner_name)||q'['
                         and a.owner       = c.object_owner
                         and a.object_type = 'PACKAGE'
                    )]' end ||q'[
-               and c.object_owner = ']'||upper(a_owner_name) ||q'['
+               and c.object_owner = ']'||upper(l_owner_name)||q'['
                and ]'
-               || case when a_package_name is not null
+               || case when l_package_name is not null
                   then 'c.object_name = :a_package_name'
                   else ':a_package_name is null' end
                || q'[
@@ -787,7 +811,7 @@ create or replace package body ut_suite_manager is
              object_owner, object_name, item_name, item_description,
              item_type, item_line_no, path, disabled_flag
            )
-      from items c]' using upper(a_package_name);
+      from items c]' using upper(l_package_name);
 
     return l_result;
   end;
@@ -798,17 +822,30 @@ create or replace package body ut_suite_manager is
     a_procedure_name varchar2 := null,
     a_item_type      varchar2 := null
   ) return boolean is
-    l_result    integer;
-    l_ut_owner  varchar2(250) := ut_utils.ut_owner;
+    l_result         integer;
+    l_ut_owner       varchar2(250) := ut_utils.ut_owner;
+    l_owner_name     varchar2(250);
+    l_package_name   varchar2(250);
+    l_procedure_name varchar2(250);
   begin
-    refresh_cache(a_owner_name);
+    if a_owner_name is not null then
+      l_owner_name := sys.dbms_assert.qualified_sql_name(a_owner_name);
+    end if;
+    if a_package_name is not null then
+      l_package_name := sys.dbms_assert.qualified_sql_name(a_package_name);
+    end if;
+    if a_procedure_name is not null then
+      l_procedure_name := sys.dbms_assert.qualified_sql_name(a_procedure_name);
+    end if;
+  
+    refresh_cache(l_owner_name);
 
     execute immediate q'[
       select count(1) from dual
        where exists (
                 select 1
                   from ]'||l_ut_owner||q'[.ut_suite_cache c
-                 where 1 = 1 ]'||case when can_skip_all_objects_scan(a_owner_name) then q'[
+                 where 1 = 1 ]'||case when can_skip_all_objects_scan(l_owner_name) then q'[
                        and exists
                            ( select 1
                                from all_objects a
@@ -820,20 +857,20 @@ create or replace package body ut_suite_manager is
                        and :a_owner_name is not null ]' end ||q'[
                        and c.object_owner = :a_owner_name
                        and ]'
-                       || case when a_package_name is not null
+                       || case when l_package_name is not null
                           then 'c.object_name = :a_package_name'
                           else ':a_package_name is null' end
                        || q'[
                        and ]'
-                       || case when a_procedure_name is not null
+                       || case when l_procedure_name is not null
                           then 'c.name = :a_procedure_name'
                           else ':a_procedure_name is null' end
                        || q'[
              )]'
       into l_result 
       using 
-        upper(a_owner_name), upper(a_owner_name),
-        upper(a_package_name), upper(a_procedure_name);
+        upper(l_owner_name), upper(l_owner_name),
+        upper(l_package_name), upper(l_procedure_name);
     
     return l_result > 0;
   end;
