2

Why is EXECUTE IMMEDIATE allowed in stored procedures, if stored procedures are meant to mitigate SQL injection attacks? The accepted answer to the following question refers to them as a step against such attacks:

What is a stored procedure? https://stackoverflow.com/a/459531/3163495

"Stored procedures also have a security benefit in that you can grant execute rights to a stored procedure but the user will not need to have read/write permissions on the underlying tables. This is a good first step against SQL injection."

...unless the stored procedure is using EXECUTE IMMEDIATE.

This PL/SQL code returns a product's description (second parameter).

CREATE OR REPLACE PROCEDURE prodDescr(vname IN VARCHAR2, vresult OUT VARCHAR2) AS
vsql VARCHAR2(4000);
BEGIN
vsql := 'SELECT description FROM products WHERE name=''' || vname || '''';
EXECUTE IMMEDIATE vsql INTO vresult;
END;

Malicious user input.

A' AND 1=2 UNION SELECT password FROM members WHERE username='admin

Generated Query.

SELECT description FROM products WHERE name='A' OR 1=2 UNION SELECT password FROM members WHERE username='admin'

When the query is executed, the attacker gets the administrator’s password.

As you can see, although we used a stored procedure, an attacker can still exploit a vulnerability just as easily as if we were an amateur developer concatenating some SELECT statement in PHP without sanitizing input. To me, it seems it can be very misleading to say to developers that stored procedures will help keep your database safe.

Community
  • 1
  • 1
user3163495
  • 2,425
  • 2
  • 26
  • 43
  • Stored procedures can help to protect against injection attacks... if you use parameterized ones. Concatenating arguments in stored procedures is just as bad as in direct queries. – Denys Séguret Apr 24 '18 at 19:09
  • Who is saying that "stored procedures" will help keep your database safe? And what are they being compared to? – Jeffrey Kemp Apr 26 '18 at 03:09
  • 1
    @JeffreyKemp see https://stackoverflow.com/a/459531/3163495, the accepted answer (by user JoshBerke). I edited my question to include the link. – user3163495 Apr 26 '18 at 15:58
  • Josh never implies that stored procedures automatically, by themselves, guarantee security. – Jeffrey Kemp Apr 26 '18 at 22:52

2 Answers2

15

Execute Immediate can still be used in a safe way. It all comes down to the logic of the stored proc. The concat is making the code unsafe not the execute immediate.

vsql := 'SELECT description FROM products WHERE name=''' || vname || '''';

Should be using bind variables or a dbms_assert call.

  vsql := 'select count(1) from all_objects where owner = :1'
  EXECUTE IMMEDIATE vsql into vresult using vname ;

OR

 vsql := 'select count(1) from all_objects where owner ='||DBMS_ASSERT.ENQUOTE_LITERAL(vname);
  EXECUTE IMMEDIATE vsql into vresult  ;

In a full example below using both methods. The first has bind(s) and the second is wrappered with DBMS_ASSERT.

SQL>declare
      v_in varchar2(2000);
      ret  varchar2(2000);
    begin
      v_in := 'KLRICE';
     EXECUTE IMMEDIATE 'select count(1) from all_objects where owner = :1' into ret using v_in ;
     dbms_output.put_line('First Object Count  : ' || ret);

     EXECUTE IMMEDIATE 'select count(1) from all_objects where owner ='||DBMS_ASSERT.ENQUOTE_LITERAL(v_in)  into ret ;

    dbms_output.put_line('Second Object Count  : ' || ret);
  end
SQL> /
First Object Count  : 74
Second Object Count  : 74


PL/SQL procedure successfully completed.

SQL> 
Kris Rice
  • 3,300
  • 15
  • 33
2

Stored procedures do not keep your database safe. That has never been true.

No language, framework, or API can keep your database safe.

It is the developer's responsibility to write safe code.

If you use EXECUTE IMMEDIATE in an unsafe way, then you have a vulnerability.

The same is true when you don't use stored procedures — if you write dynamic SQL using any application language, you have the same risk of creating SQL injection vulnerabilities.

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828