CODEWITHSUNDEEP
oracle-databaseplsql
Karan Khanna
Karan Khanna

Reputation: 2147

Security issues with SQL Query

I am working on an Oracle SQL query. The query is getting flagged by Fortify SCA for Privilege Management: Default Function or Procedure Rights. Can someone help me with the correct way of using the query?

The query I want to use:

CREATE OR REPLACE PROCEDURE "reset_sequence"
IS
  l_value NUMBER;
BEGIN
  EXECUTE IMMEDIATE 'SELECT "ordering_seq".nextval FROM dual' INTO l_value;
  EXECUTE IMMEDIATE 'ALTER SEQUENCE "ordering_seq" INCREMENT BY -' || l_value || ' MINVALUE 0';
  EXECUTE IMMEDIATE 'SELECT "ordering_seq".nextval FROM dual' INTO l_value;
  EXECUTE IMMEDIATE 'ALTER SEQUENCE "ordering_seq" INCREMENT BY 1 MINVALUE 0';
END;

Upvotes: 1

Views: 1741

Answers (2)

Chris Saxon
Chris Saxon

Reputation: 9865

I can spot a couple of issues here:

  • There's no authid clause, so it defaults to definer
  • execute immediate with string concatenation

This means anyone with execute privileges on the procedure is running with the full rights of the procedure owner. And with string concatenation, there's the risk of SQL injection. Yes, even with numbers.

Also you can get the next value of a sequence by assigning it. No need for execute immediate.

To be safe, I'd make the following changes:

  • Add authid current_user
  • Explicitly to_char the increment, avoiding attacks on this

Giving:

create sequence ordering_seq
  start with 100;

select ordering_seq.nextval from dual;

NEXTVAL   
       100 

CREATE OR REPLACE PROCEDURE reset_sequence
  authid current_user
IS
  l_value NUMBER;
BEGIN
  l_value := ordering_seq.nextval;
  EXECUTE IMMEDIATE 'ALTER SEQUENCE ordering_seq INCREMENT BY -' || 
     to_char ( l_value, 'TM', 'NLS_Numeric_Characters = ''.,''' ) || 
     ' MINVALUE 0';
  l_value := ordering_seq.nextval;
  EXECUTE IMMEDIATE 'ALTER SEQUENCE ordering_seq INCREMENT BY 1 MINVALUE 0';
END;
/

exec reset_sequence;

select ordering_seq.nextval from dual;

NEXTVAL   
         1

Of course, using invoker's rights means you have to give out alter sequence rights to whoever calls. Which brings its own issues. To overcome this you could use Code-Based Access Control.

Upvotes: 4

Alex Poole
Alex Poole

Reputation: 191425

From the Fortify docs:

Privilege Management: Default Package Rights

PLSQL/TSQL

Abstract

Packages without an AUTHID clause default to AUTHID DEFINER.

Explanation

PL/SQL packages can be either AUTHID DEFINER or AUTHID CURRENT_USER. Functions and procedures in a package with definer's rights execute under the privileges of the user that defines the package. This can allow updates and access to specific pieces of data without granting access to entire tables or schemas. In a package with invoker's rights, or AUTHID CURRENT_USER, functions and procedures execute under the privileges of the user who invokes them. This does not allow a user to gain access to data it didn't already have access to. If no AUTHID clause is provided, the package defaults to definer's rights.

Packages are usually defined by SYS or another highly privileged user, making any exploits of the code potentially more dangerous.

So it seems you just need to add an AUTHID clause, even if that is just explicitly stating the default value again (though you should establish the correct value, of course).

Not relevant, but neither select needs to be dynamic - you may have chosen to do that so it looks more consistent with the alter statements, but it isn't necessary; and they don't even need to be selects any more - you could do:

CREATE OR REPLACE PROCEDURE "reset_sequence"
AUTHID CURRENT_USER
IS
  l_value NUMBER;
BEGIN
  l_value := "ordering_seq".nextval;
  EXECUTE IMMEDIATE 'ALTER SEQUENCE "ordering_seq" INCREMENT BY -' || l_value || ' MINVALUE 0';
  l_value := "ordering_seq".nextval;
  EXECUTE IMMEDIATE 'ALTER SEQUENCE "ordering_seq" INCREMENT BY 1 MINVALUE 0';
END;

Upvotes: 3

Related Questions