How to prevent SQL Injection in PL/SQL

Question

We have some few packages where we need to resolve some SQL Injection issues. I need some help to rewrite sql statement or sanitize the inputs. Below is the line number where veracode throw the error.

open c_ccl (p_part_nr,p_ctry_cd);

// Source code

     CREATE OR REPLACE EDITIONABLE PACKAGE BODY "schema"."Test_PKG" AS

  v_data t_cla_class_data;

  FUNCTION nat_eccn_cd( p_part_nr  IN    t_part_nr, p_ctry_cd    IN     t_ctry_cd         )            
  RETURN t_us_eccn_cd IS
  CURSOR c_ccl(p_part_nr CHAR, p_ctry_cd CHAR)  IS
  SELECT NAT_CCL_CD  FROM CLSDBA.CLA_EXP_PART_CTRY e
     WHERE  e.PART_NR = p_part_nr  AND e.CTRY_CD = p_ctry_cd
      ORDER BY e.VAL_FROM_DT DESC;
  v_ctry_cd char(4) := p_ctry_cd;
  v_trf_cd char(4);
  BEGIN
    v_data.nat_eccn_cd := NULL;
    open c_ccl (p_part_nr,p_ctry_cd);
    fetch c_ccl INTO v_data.nat_eccn_cd;
    close c_ccl;
    return (trim(v_data.nat_eccn_cd));
    exception when others then return NULL;
  end;

I don't see any SQL injection either but your error handling is a nightmare waiting to happen. Do you really not care about any error that could get raised? It's likely that you want to ignore some errors but not all, the ones you can't handle should be raised. You also want to be making sure you are closing your cursors if an error does happen - the default behaviour of not having an exception when others, would do this for you. — Andrew Sayer
– Andrew Sayer, Commented Oct 25, 2020 at 18:34
The way how you write the function is exactly the way to prevent any SQL injection. What is your problem? — Wernfried Domscheit
– Wernfried Domscheit, Commented Oct 25, 2020 at 21:21

Jon Heller · Accepted Answer · 2020-10-25 19:03:58Z

1

I don't see any SQL injection issues with your code - there is no dynamic code where the user inputs could be evaluated and escape out of the expected code flow. Unless your code snippet is generated somewhere else, or one of the column names is really a function that calls dynamic SQL, your code looks safe.

You used the phrase "sanitize the inputs", which is terrible advice for database programming. As much as I love the comic strip XKCD, Randall got this one wrong.

Bind variables are the best solution to avoiding SQL injection. I'll take this opportunity to (poorly) change his comic:

answered Oct 25, 2020 at 19:03

Jon Heller

37k6 gold badges84 silver badges150 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Dips Over a year ago

Hi, I can understand but i have scan again with veracode software and it's showing sql injection error in below line. open c_ccl (p_part_nr,p_ctry_cd); Is it possible because of variable passing to cursor?

Erich Kitzmueller Over a year ago

Apparently, veracode is wrong. It can be a huge pain in the ass to make such a tool happy when there is in fact nothing at all wrong with your code. The best thing you can do is to file a bug report to the developers of veracode. Maybe they can explain why veracode shows a vulnerability when there is none, and with some luck, you can mark the code as safe with a special comment or so to work around that bug in veracode.

Collectives™ on Stack Overflow

How to prevent SQL Injection in PL/SQL

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related