Subscribe

Share

Database, SQL and PL/SQL

Dynamically Dangerous Code

There’s a right time to use dynamic SQL, but there’s never a right time for SQL injection.

By Steven Feuerstein Oracle ACE Director

May/June 2015

“I’ve got a real problem on my hands, Steven,” he said. “I followed your advice to create reusable program units rather than one-offs with similar functionality. But now my program is raising errors that I can’t sort out and doing things I never intended.”


Answer to Last Issue’s Challenge
The PL/SQL Challenge quiz in last issue’s “Four Resolutions for Better Code” presented three code blocks and asked which block(s) would display “-6502” after execution. All three answers are correct, but only the first (a) follows the native PL/SQL paradigm for error raising and handling. The other two techniques should, therefore, be avoided.

It’s never good to hear a programmer worrying about a program with a will of its own, so I hopped into my bright-red PLSQLmobile and raced over to Bob’s cubicle.

“Take a look at this,” said Bob. “I’m using my program in our HR system. You’re familiar with the employees table, right? So let’s update the salary of employee 100.”

BEGIN
em_update_col_value ('employees',
'employee_id',
100,
'salary',
1000);
END;
/
Value of salary updated to 1000

“No problem, right? OK, now let’s try to update the department name and...kaboom!”

BEGIN
em_update_col_value ('departments',
'department_id',
10,
'department_name',
'Jolly Fun');
END;
/
ORA-00933: SQL command not properly ended
ORA-06512: at "SYS.DBMS_SQL", line 1053
ORA-06512: at "QDB_BETA
.EM_UPDATE_COL_VALUE", line 11
ORA-06512: at line 2

“How,” wondered Bob, with a pained expression on his face, “can it work for one column and not another?”

Without even glancing at Bob’s code, I already had a pretty good idea of the problem—or problems. I grabbed his keyboard and typed.

“And how about this?” I asked.

BEGIN
em_update_col_value ('employees',
'employee_id=employee_id;
delete from employees
where employee_id',
100,
'salary',
1000);
END;
/
SELECT * FROM employees
WHERE employee_id = 100
/
No rows found

“Exactly!” shouted Bob, pointing at the screen. “What’s with that? How can my program delete a row from a table when all it contains is an UPDATE statement?”

To Bob I merely said, “Let’s take a look.”


“All” It Does Is an Update?

Even before looking at the em_update_col_value procedure, I was pretty certain of a few things, based on what I’d just witnessed:

  • The procedure was executing dynamic SQL.
  • Bob’s error handling was minimal or nonexistent.
  • Bob had taken no precautions against SQL injection.

But I must admit that I was not quite prepared for the awfulness that presented itself to me when Bob opened em_update_col_value in Oracle SQL Developer, as shown in Listing 1.

Code Listing 1: The original—and awful—em_update_col_value procedure

PROCEDURE em_update_col_value (
table_in IN VARCHAR2,
pkey_col_in IN VARCHAR2,
pkey_value_in IN INTEGER,
update_col_in IN VARCHAR2,
value_in IN VARCHAR2)
IS
l_cursor PLS_INTEGER := DBMS_SQL.open_cursor;
l_feedback PLS_INTEGER;
BEGIN
DBMS_SQL.parse (
l_cursor,
'BEGIN update '
|| table_in
|| ' set '
|| update_col_in
|| ' = '
|| value_in
|| ' where '
|| pkey_col_in
|| ' = '
|| pkey_value_in
|| '; END;',
DBMS_SQL.native);
l_feedback := DBMS_SQL.execute (l_cursor);
IF l_feedback > 0
THEN
DBMS_OUTPUT.PUT_LINE (
'Value of '
|| update_col_in
|| ' updated to '
|| value_in);
ELSE
DBMS_OUTPUT.PUT_LINE (
'Update of '
|| update_col_in
|| ' to '
|| value_in
|| ' failed.');
END IF;
DBMS_SQL.close_cursor (l_cursor);
END;
/

“See?” said Bob, “No deletes. Just an update. A nice generic procedure for executing an update against any column in any table. Really neat, huh?”

I decided to break the news gently. “Bob, this procedure is a total abomination, but I like your energy and enthusiasm.”

I shared my questions and concerns with him:

  • Is this program too reusable to be useful?
  • Why is it using DBMS_SQL?
  • Where’s the error handling?
  • Your program is wide open to SQL injection.

“But don’t feel bad, Bob,” I concluded. “This will be a great learning moment. Shall we explore?” Bob nodded a bit glumly.


When Is Reusable Too Reusable?

“Tell me why you wrote this procedure,” I started off.

Bob recounted to me a session from one of my trainings: “You urged us to find every opportunity to reuse code instead of writing the same or similar code in multiple places. I noticed that in at least five places in our code, we executed updates of a single column in a single row. So I figured I could write a single procedure with dynamic SQL and that we then could call that one procedure instead of writing updates over and over again.”

Bob’s justification sounded reasonable on the surface, but in fact it was a big mistake.

It is better to reuse code whenever possible, but only when that is appropriate. There are several reasons this guideline does not apply to single-column updates: dynamic SQL is more complex than static SQL, it executes more slowly than static SQL, it’s harder to make secure, and it’s harder to debug.

So you want to use it only when absolutely necessary—and that is certainly not the case here. In em_update_col_value, dynamic SQL was used for the sake of convenience.

“When it comes to writing SQL in your PL/SQL code,” I admonished Bob, “you should use dynamic SQL only when it is required—when a user needs to supply some additional information at runtime to complete the SQL statement.

“So I suggest that you abandon em_update_col_value and instead go to wherever the procedure is called and replace it with a new procedure call, such as em_update_salary or em_update_last_name.”

“But then I end up with dozens of different procedures!” Bob exclaimed. “Why not just execute the UPDATE directly in my code?”

“You could do that, but if you put each UPDATE inside a procedure, then it is possible you will reuse that procedure—and not duplicate the UPDATE statement,” I responded. “You are also more likely to write better error handling, and you can add functionality to the update later—in just one place—as your requirements change.”

Bob nodded sadly. I could tell he didn’t like having to throw away his generic procedure.

“But I tell you what, Bob: let’s still go through this procedure and draw out some lessons for the right way to construct a procedure that relies on dynamic SQL. You are sure to run into the need soon.” Bob brightened, and off we went.


Why Are You Using DBMS_SQL?

“First of all,” I told Bob, “let’s do some basic cleanup in your program so that it is easier to focus on the bigger issues. You should use DBMS_SQL only if you have very complex requirements, such as not knowing at compile time how many columns you are querying or how many variables you must bind. Because that is not the case here, EXECUTE IMMEDIATE is a better fit.

“In addition, your IF statement after the UPDATE is verbose and distracting.” I tapped at the keyboard for a minute or two. “Here. What do you think of this?” I asked, pointing to the code in Listing 2.

Code Listing 2: The updated—but still flawed—em_update_col_value procedure

PROCEDURE em_update_col_value (
table_in IN VARCHAR2,
pkey_col_in IN VARCHAR2,
pkey_value_in IN INTEGER,
update_col_in IN VARCHAR2,
value_in IN VARCHAR2)
IS
PROCEDURE report_results
IS
BEGIN
DBMS_OUTPUT.PUT_LINE (
'Value of '
|| update_col_in
|| CASE SQL%ROWCOUNT WHEN 0 THEN ' NOT' END
|| ' updated to '
|| value_in);
END;
BEGIN
EXECUTE IMMEDIATE
'BEGIN update '
|| table_in
|| ' set '
|| update_col_in
|| ' = '
|| value_in
|| ' where '
|| pkey_col_in
|| ' = '
|| pkey_value_in
|| '; END;';
report_results;
END;

“Oh, right,” said Bob, “you used a nested subprogram to hide the reporting details. I like the inline CASE expression, too. And really that’s all I need to do with EXECUTE IMMEDIATE? All that other code disappears?”

“That’s correct,” I replied. “There’s no need to declare and manage cursors, no need to parse and then execute. That’s all taken care of for us. Nice, eh?”


Where’s Your Error Handling?

“We’re still far from done, though,” I pointed out. “Right now this program assumes that everything is going to proceed without any problem. What’s the chance of that happening? Assume there will be an error. What can we do, then, to make it easier to figure out what went wrong and fix it?”

The challenge with most dynamic SQL requirements is usually not figuring out how to use EXECUTE IMMEDIATE; it’s a simple, elegant statement. No, programmers are much more likely to run into problems constructing the dynamic SQL at runtime. The smallest mistake (forgetting to leave a space between keywords, for example) results in SQL that cannot be parsed.

So any program that contains dynamic SQL should do the following:

  • Assign the dynamically constructed SQL statement to a variable and then use EXECUTE IMMEDIATE on that variable
  • Add an exception handler that logs the error along with the variable containing the SQL statement
  • Reraise the exception so that the calling program knows that something went wrong

Applying these principles, Bob and I updated the em_update_col_value procedure to the code in Listing 3.

Code Listing 3: The further updated em_update_col_value procedure, with error handling

PROCEDURE em_update_col_value (
table_in IN VARCHAR2,
pkey_col_in IN VARCHAR2,
pkey_value_in IN INTEGER,
update_col_in IN VARCHAR2,
value_in IN VARCHAR2)
IS
l_statement VARCHAR2 (32767);
PROCEDURE report_results ...
BEGIN
l_statement :=
'BEGIN update '
|| table_in
|| ' set '
|| update_col_in
|| ' = '
|| value_in
|| ' where '
|| pkey_col_in
|| ' = '
|| pkey_value_in
|| '; END;';
EXECUTE IMMEDIATE l_statement;
report_results;
EXCEPTION
WHEN OTHERS
THEN
em_error_log_pkg.log_error (l_statement);
RAISE;
END;

The new em_error_log_pkg.log_error procedure (called in the updated em_update_col_value procedure) should write out to a log table all of the following:

  • SQLCODE – the current error code
  • DBMS_UTILITY.FORMAT_ERROR_STACK – the current error message and/or stack (I recommend that you use this instead of SQLERRM)
  • DBMS_UTILITY.FORMAT_CALL_TRACE – the execution call stack, answering the question “How did I get here?”
  • DBMS_UTILITY.FORMAT_ERROR_BACKTRACE – the trace back to the line number on which the error was raised
  • Any information passed to the procedure by the application developer (in this case, the value of l_statement)

Assume for this article, however, that the log_error procedure simply displays the value of l_statement with a call to DBMS_OUTPUT.PUT_LINE.

I asked Bob to run the following block again, against the latest version of the em_update_col_value procedure, to see if it would help with the “ORA-00933: SQL command not properly ended” error message he received when he ran the block against the first version of em_update_col_value. Bob ran

BEGIN
em_update_col_value ('departments',
'department_id',
10,
'department_name',
'Jolly Fun');
END;
/

And then we saw this output, in addition to the error stack:

BEGIN update departments 
set department_name = Jolly Fun
where department_id = 10; END;

“D’oh!” Bob groaned. “Well that’s obvious enough—now that I can see the string. I forgot to put single quotes around the value. When it’s a number, no problem. When the value is a string, big problem!”

Bob grabbed the keyboard and a moment later had the problem “fixed.”

   l_statement :=
'BEGIN update '
|| table_in
|| ' set '
|| update_col_in
|| ' = '''
|| value_in
|| ''' where '
|| pkey_col_in
|| ' = '''
|| pkey_value_in
|| '''; END;';

I nodded. “Yep, that fixes the specific problem caused by string values, but the main lesson here is this: assign that expression to a variable so you can easily trace and log the value. You will then be able to diagnose the problem—and achieve the proper dynamic SQL statement construction—much more quickly.”

Bob smiled. One less bug to worry about. Then he frowned. “But what about the deletion from my table? That was really weird, and I have a feeling we haven’t fixed that yet.”


What IS SQL Injection?

“You read my mind, Bob. That’s right. That problem—and it is far and away the most serious problem with your procedure—still lurks. And it has a name: SQL injection.

“SQL injection occurs when users insert their own text into your SQL statement and cause it to do things you never intended—such as delete a row.”

“Two questions: How could that possibly happen, and how do I make sure it can’t happen?” Bob asked.

“Right,” I responded. “Let’s go back to that delete example I gave you and run it with our new, error-handling-enriched version of the em_update_col_value procedure, but this time I will comment out the semicolon (;) before the delete to force an error.”

BEGIN
em_update_col_value ('employees',
'employee_id=employee_id /*;*/
delete from employees
where employee_id',
100,
'salary',
1000);
END;
/
BEGIN update employees set salary = 1000
where employee_id=employee_id
/*;*/ delete from employees
where employee_id = 100; END;

“With a semicolon just before the DELETE keyword, a malicious user terminates the UPDATE statement (which is setting everyone’s salary to 1000) and then starts a brand-new statement inside the block, performing a DELETE. Those semicolons embedded in PL/SQL blocks can really wreak havoc!”

Bob sighed and nodded. “OK, now I see what is going on. What can I do about it?”

“First, Bob, I need to set expectations properly. SQL injection is a security issue. This means that you need to engage with your chief security officer to make sure you are following all of extremememe’s guidelines. It is also a very specialized topic, and I am not a security specialist. So I will share with you some basic steps you should take to shore up your defenses, but I also encourage you to check out the excellent ‘How to write SQL injection proof PL/SQL’ white paper, available on Oracle Technology Network.”

I then presented the following concerns regarding the latest version of the em_update_col_value procedure to Bob:


  1. The procedure includes unnecessary construction and execution of a dynamic PL/SQL block.
  2. Users can pass their own strings directly to the procedure.
  3. The procedure does not check to make sure the table or column names are valid.

I elaborated on each of these to Bob:


  1. Dynamic PL/SQL—a string that starts with “DECLARE” or “BEGIN” and ends with “END;”—is much more vulnerable to injection than dynamic SQL (a data manipulation language [DML] or data definition language [DDL] statement), because you can execute procedural logic, invoke stored program units, and so on.

    So if you are not actually executing PL/SQL code, do not put your SQL statements inside a PL/SQL block. In the em_update_col_value procedure, the assignment to the local variable should be nothing more than

       l_statement :=
    'update '
    || table_in
    || ' set '
    || update_col_in
    || ' = '''
    || value_in
    || ''' where '
    || pkey_col_in
    || ' = '''
    || pkey_value_in
    || '''';
  2. An end user should never be able to directly insert text into a string executed dynamically. If this is allowed, it will always be very difficult to stop injection.

    User inputs should be tightly constrained and then checked before they are used in a dynamically constructed string. There is no general solution for performing this task. You must analyze each use case and decide how to guard your database from injection. The first step is to avoid concatenation whenever possible and instead bind variable values into the string. You cannot inject into variables! I updated em_update_col_value to use bind variables and showed it to Bob:

    BEGIN
    l_statement :=
    'UPDATE '
    || table_in
    || ' SET '
    || update_col_in
    || ' = :my_value WHERE '
    || pkey_col_in
    || ' = :my_pky';
    EXECUTE IMMEDIATE l_statement
    USING value_in, pkey_value_in;

    “OK,” said Bob. “I see now that users can’t inject through the values, but what about the table and column names?”

    “Right,” I responded. “That brings us to the final point.”

  3. This procedure accepts the name of a table and a column and then concatenates them directly into the string. You cannot bind a table name into a SQL statement with the USING clause; the SQL engine needs all that information before binding to ensure that it is a valid SQL statement. Theoretically, injection could still occur if there is concatenation.

“So,” I explained, “first make sure that users can never enter a table name or a column name directly. It sounds unlikely that they’d be able to, doesn’t it? But make sure! Next, you can further guard against injection via object names by using DBMS_ASSERT subprograms to check that the string is the name of a database object and/or is a valid object name.”

l_statement :=
'UPDATE '
|| DBMS_ASSERT.SQL_OBJECT_NAME (
table_in)
|| ' SET '
|| DBMS_ASSERT.SIMPLE_SQL_NAME (
update_col_in)
|| ' = :my_value WHERE '
|| DBMS_ASSERT.SIMPLE_SQL_NAME (
pkey_col_in)
|| ' = :my_pky';

“So now with a call to DBMS_ASSERT.SQL_OBJECT_NAME, if I pass a ‘bad’ name for the table, I will see

BEGIN
em_update_col_value(
'employees; more code here',
'employee_id',
1000000,
'salary',
1000);
END;
/
ORA-44002: invalid object name

“And if I try to play games with the column name, Oracle Database will reject my effort.”

BEGIN
em_update_col_value(
'employees',
'employee_id;more code here',
1000000,
'salary',
1000);
END;
/
ORA-44003: invalid SQL name

Bob smiled broadly. “I like it when Oracle Database takes care of the heavy lifting for me.”

“Indeed,” I agreed. “But when it comes to SQL injection, Oracle Database makes no promises. You need to do the lion’s share of the work to ensure that your code is not vulnerable to SQL injection.”


Dynamic and Reusable? An Unlikely Pair

Programmers should always strive for a single point of definition (usually a subprogram) for rules, formulas, SQL statements, and magic values. Reuse those subprograms, and look for opportunities to build generic utilities, such as error loggers, that can be reused throughout an application.

Nevertheless, programs that execute dynamic SQL statements are unlikely to be a good fit for reusable code. Dynamic SQL should be utilized only when static implementations are impossible. And when you write a subprogram with dynamic SQL, the need for solid error handling and proactive protection against SQL injection rises significantly.


Take the Challenge

Each PL/SQL article offers a quiz to test your knowledge of the information provided in it. The quiz appears below and also at PL/SQL Challenge (plsqlchallenge.com), a website that offers online quizzes on the PL/SQL language as well as SQL, Oracle Application Express, and database design.

Here is your quiz for this article:

I execute the following statements:

CREATE TABLE plch_persons
( pky INTEGER PRIMARY KEY,
nm VARCHAR2 (100))
/
CREATE TABLE plch_trees
( pky INTEGER PRIMARY KEY,
nm VARCHAR2 (100))
/
BEGIN
INSERT INTO plch_persons
VALUES (1, 'Sam');
INSERT INTO plch_trees
VALUES (1, 'Oak');
COMMIT;
END;
/

Which of the following choices create(s) a procedure named PLCH_SHOW_NAME so that after the following block executes, both “Oak” and “Sam” are displayed on the screen?

BEGIN
plch_show_name ('PLCH_TREES', 1);
plch_show_name ('PLCH_PERSONS', 1);
END;
/

a.

CREATE OR REPLACE PROCEDURE 
plch_show_name (table_in IN VARCHAR2, pky_in IN INTEGER)
IS
BEGIN
EXECUTE IMMEDIATE
'DECLARE l_value VARCHAR2(100);
BEGIN SELECT nm INTO l_value
FROM '
|| table_in
|| ' WHERE pky = '
|| pky_in
|| '; DBMS_OUTPUT.PUT_LINE (l_value);
END;';
END;
/

b.

CREATE OR REPLACE PROCEDURE 
plch_show_name (table_in IN VARCHAR2, pky_in IN INTEGER)
IS
l_value VARCHAR2 (100);
BEGIN
EXECUTE IMMEDIATE
'BEGIN SELECT nm INTO :val FROM '
|| table_in
|| ' WHERE pky = '
|| pky_in
|| '; END;'
USING OUT l_value;
DBMS_OUTPUT.PUT_LINE (l_value);
END;
/

c.

CREATE OR REPLACE PROCEDURE 
plch_show_name (table_in IN VARCHAR2, pky_in IN INTEGER)
IS
l_value VARCHAR2 (100);
BEGIN
EXECUTE IMMEDIATE
'SELECT nm FROM ' || table_in || ' WHERE pky = ' || pky_in
INTO l_value;
DBMS_OUTPUT.PUT_LINE (l_value);
END;
/

d.

CREATE OR REPLACE PROCEDURE 
plch_show_name (table_in IN VARCHAR2, pky_in IN INTEGER)
IS
l_value VARCHAR2 (100);
BEGIN
EXECUTE IMMEDIATE
'SELECT nm FROM '
|| DBMS_ASSERT.sql_object_name (table_in)
|| ' WHERE pky = :pky'
USING pky_in
INTO l_value;
DBMS_OUTPUT.PUT_LINE (l_value);
END;
/

Next Steps

 TEST your PL/SQL knowledge

READ more Feuerstein
 bit.ly/omagplsql
 stevenfeuersteinonplsql.blogspot.com

READ more about
 PL/SQL
SQL injection
 How to write SQL injection proof PL/SQL



Photography by Ricardo Gomez, Unsplash