Code can be scary when you simplify it

Disclaimer: I’m not posting to make me look better, we’ve all written code that we’re later ashamed of, and I’m no different!

This is some code I discovered buried in a system some time ago. I’ve kept a copy of it because it illustrates a number of things NOT to do:

FUNCTION password_is_valid
  (in_password IN VARCHAR2)
-- do NOT copy this code!!! ...
  RETURN VARCHAR2 IS
  l_valid VARCHAR2(1);
  l_sql VARCHAR2(32000);
  CURSOR cur_rules IS
    SELECT REPLACE(sql_expression
                  ,'#PASSWORD#'
                  ,'''' || in_password || ''''
                  ) AS sql_expression
    FROM password_rules;
BEGIN
  FOR l_rec IN cur_rules LOOP
    l_valid := 'N';
    -- SQL injection, here we come...
    l_sql := 'SELECT ''Y'' FROM DUAL ' || l_rec.sql_expression;
    BEGIN
      -- why not flood the shared pool with SQLs containing
      -- user passwords in cleartext?
      EXECUTE IMMEDIATE l_sql INTO l_valid;
    EXCEPTION
      WHEN NO_DATA_FOUND THEN
        EXIT;
    END;
    IF l_valid = 'N' THEN
      EXIT;
    END IF;
  END LOOP;
  RETURN l_valid;
END password_is_valid;

I am pretty sure this code was no longer used, but I couldn’t be sure as I didn’t have access to all the instances that could run it.

2 thoughts on “Code can be scary when you simplify it

Comments are closed.