Unable to fix : ORA-00907: missing right parenthesis-CodePudding

I have a syntax issue and I can't sort it out :

CREATE OR REPLACE FUNCTION Checker(table_name VARCHAR, names VARCHAR) 
RETURN VARCHAR
AS
    result VARCHAR2(255);
    sql_query VARCHAR2(32767);
    BEGIN
        sql_query := 'SELECT StringType INTO result FROM (
            SELECT StringType, COUNT(*) as num_occurrences FROM ' || table_name || '
            WHERE titre = ''' || names || '''
            GROUP BY StringType
            ORDER BY num_occurrences DESC
            LIMIT 1
        ) as temp';
        EXECUTE IMMEDIATE sql_query;
        RETURN result;
    END;
 /   
    
select Checker('Table_Name', 'WantedName') as blabla from dual;

Why does this throw : Error starting at line : 19 in command - select Checker('Table_Name', 'WantedName') as blabla from dual Error report - ORA-00907: missing right parenthesis ORA-06512: at "Checker", line 14

The goal of this function is to find StringType which occur the most for a given table Table_Name with a certain name WantedName.

What am I doing wrong ?

CodePudding user response：

There is no SELECT INTO in SQL. It is part of the PL/SQL programming language and must hence be done outside the executed SQL: EXECUTE IMMEDIATE sql_query INTO result;

There also is no LIMIT in standard SQL and Oracle. It must be FETCH FIRST ROW ONLY instead.

The complete corrected code:

CREATE OR REPLACE FUNCTION Checker(table_name VARCHAR2, names VARCHAR2) 
RETURN VARCHAR2
AS
  result VARCHAR2(255);
  sql_query VARCHAR2(32767);
BEGIN
  sql_query := 
     'SELECT stringtype
      FROM ' || table_name || '
      WHERE titre = ''' || names || '''
      GROUP BY stringtype
      ORDER BY COUNT(*) DESC
      FETCH FIRST ROW ONLY';
  EXECUTE IMMEDIATE sql_query INTO result;
  RETURN result;
END;
/

Demo: https://dbfiddle.uk/NybrAFNW

(You should follow some naming convention, by the way. E.g. precede variables with v_ and parameters with p_. This makes PL/SQL code more readable and less prone to errors.)

CodePudding user response：

NEVER use string concatenation to build queries, that is how you introduce SQL injection vulnerabilities. Instead use bind variables, wherever possible, and sanitize the input where you cannot:

CREATE FUNCTION Checker(
  table_name IN VARCHAR2,
  names      IN VARCHAR2
) RETURN VARCHAR
AS
  result VARCHAR2(255);
  sql_query VARCHAR2(32767);
BEGIN
  sql_query := 'SELECT StringType
                FROM   ' || DBMS_ASSERT.SQL_OBJECT_NAME(table_name) || '
                WHERE titre = :names
                GROUP BY StringType
                ORDER BY COUNT(*) DESC
                FETCH FIRST ROW ONLY';
  EXECUTE IMMEDIATE sql_query INTO result USING names;
  RETURN result;
END;
/

Then, if you have he sample data:

CREATE TABLE table_name (stringtype, titre) AS
SELECT 'a', 'name1' FROM DUAL UNION ALL
SELECT 'b', 'name1' FROM DUAL UNION ALL
SELECT 'b', 'name1' FROM DUAL UNION ALL
SELECT 'a', 'name2' FROM DUAL UNION ALL
SELECT 'b', 'name2' FROM DUAL;

CREATE TABLE secret_table (username, password_hash) AS
SELECT 'admin',     '0123456789abcdef' FROM DUAL UNION ALL
SELECT 'superuser', 'password'         FROM DUAL;

Then:

SELECT Checker('TABLE_NAME', 'name1') FROM DUAL;

Outputs:

CHECKER('TABLE_NAME','NAME1')
b

But if you try to perform SQL injection attacks such as:

SELECT Checker(
         'Table_Name, secret_table',
         'WantedName'' AND username = ''admin'' AND password_hash = ''0123456789abcdef'
       ) AS throw_error
FROM   DUAL;

Gives the exception:

ORA-44002: invalid object name

and:

SELECT Checker(
         'Table_Name',
         ''' OR EXISTS(SELECT 1 FROM secret_table WHERE username = ''admin'' AND password_hash = ''0123456789abcdef'') AND ''1'' = ''1'
       ) AS doesnt_work
FROM   DUAL;

Passes the names as a literal and does not execute it in the query so outputs:

DOESNT_WORK
null

fiddle