Thursday 8 September 2011

Is searching for the string 1=1 a security risk

Question:

Hi Steven, thanks for enlightening us with your great tips on OTN.
I found something that I think does not make sense, however, in your query:
'SELECT COUNT(*) FROM ' || tab || ' WHERE ' || NVL (whr, '1=1');
Many security products that analyze packets search for conditions such as 1=1 when checking for cases of SQL injection. For that reason, I don't think it's a good idea to have such a statement in your own application. At the moment I am working to make all our databases SOX compliant, and the consultants suggest that we analyze Oracle packets to search for SQL injection. The example they gave was this situation precisely (searching for the string 1=1).

Answer:

Aldo, you raise an excellent point. I have long used "1 = 1" as a lazy way of handling a NULL dynamic WHERE clause, as you see in the code below:
CREATE OR REPLACE FUNCTION tabcount (
tab IN VARCHAR2
, whr IN VARCHAR2 := NULL)
RETURN PLS_INTEGER AUTHID CURRENT_USER
IS
str VARCHAR2 (32767);
retval PLS_INTEGER;
BEGIN
str := 'SELECT COUNT(*) FROM ' || tab ||
' WHERE ' || NVL (whr, '1=1');

EXECUTE IMMEDIATE str INTO retval;

RETURN retval;
END;
/

Given the heightened concerns about security and the knowledge that this string raises warning flags, I plan to change my slovenly ways and write just a little bit more code to avoid that string, as in:
CREATE OR REPLACE FUNCTION tabcount (
tab IN VARCHAR2
, whr IN VARCHAR2 := NULL)
RETURN PLS_INTEGER AUTHID CURRENT_USER
IS
str VARCHAR2 (32767) := 'SELECT COUNT(*) FROM ' || tab;
retval PLS_INTEGER;
BEGIN
IF whr IS NOT NULL
THEN
str := str || ' WHERE ' || whr;
END IF;

EXECUTE IMMEDIATE str
INTO retval;

RETURN retval;
END;
/
Hmmm. Well, of course, all I have done is reduce the chance that this program will be tagged as a security risk. It still does, however, offer the possibility of injection of a potentially dangerous string into my query. So I offer the following final reflections on this situation:
• Generic utilities like tabcount are not end-user programs. They are called by other developers, and are therefore not as vulnerable to intrusion in that they will generally not be invoked directly in response to user input.
• Whenever possible, avoid concatenation of chunks of SQL text and instead rely on placeholders and bind variables. This isn't easy to do with very generic programs like tabcount, but most of you will be writing programs that are more specific to your application requirements.
Consider the following program. It employs the USING clause and placeholders to update a column named salary for the specified (dynamic) table:
CREATE OR REPLACE PROCEDURE upd_salary (
tab_in IN VARCHAR2,
start_in IN DATE,
end_in IN DATE,
val_in IN NUMBER
)
IS
BEGIN
BEGIN
EXECUTE IMMEDIATE
'UPDATE '
|| tab_in
|| ' SET salary = :val WHERE hiredate BETWEEN :lodate AND :hidate'
USING val_in, start_in, end_in;

DBMS_OUTPUT.put_line ('Rows updated: ' || TO_CHAR (SQL%ROWCOUNT));
END;
/;
We could have concatenated the values of salary, low date, and high date into the string. The resulting code would, however, be more vulnerable to corruption/injection—and be much harder to read and maintain.

No comments:

Post a Comment