SQL Injection vulnerability using int parameter

Question

Most of the time I access data using stored procedures but at times I use statements which I believe are not vulnerable to SQL injection.

Below is an example I use

protected void Page_Load(object sender, EventArgs e)
{
        try
        {
            int  CatID = Request["CatID"];

            if (!IsPostBack)
            {
                getDetails(CatID);
           }
        }
        catch (Exception ex)
        {
            Response.Write(ex.Message.ToString());
        }
    }

    private DataTable getDetails( int CatID)
    {
        try
        {
            DataSet ds = new DataSet();

            string strSql = "SELECT * FROM TableXYZ WHERE CatID = "+CatID ;
            ds = DataProvider.Connect_Select(strSql);
            DataTable dt = ds.Tables[0];
            return dt;
        }
        catch (Exception ex)
        {
            throw;
        }
    }

I filter my input or query string and then I call getDetails function and pass CatID as parameter to the function & then to SQL statement. Since this is an integer type data is this code vulnerable to SQL injection?

I want to clear my doubt so that I don't use SQL statement like this.

It's not vulnerable to injection but you should use a parameter anyway for the benefits of easier query plan caching. — ta.speot.is
– ta.speot.is, Commented Nov 25, 2013 at 8:56
I would always be very cautious about appending content from the user directly to the SQL, unless you filter it heavily (for example in this case, you could do an Int.TryParse(CatID) first to make sure it's just a number, which helps you avoid crashing if they mess up). Even then, why bother filtering it and having to worry about that when you can simply use SqlParameters? — Tobberoth
– Tobberoth, Commented Nov 25, 2013 at 8:56
@ta.speot.is: "plain SQL strings" can just as well have their query plans cached. @Tobberoth: Why do an Int.Parse(CatID)? CatID is already (guaranteed to be!) an int (see method signature). — RobIII
– RobIII, Commented Nov 25, 2013 at 9:01
@RobIII Did I say they can't? you should use a parameter anyway for the benefits of easier query plan caching. — ta.speot.is
– ta.speot.is, Commented Nov 25, 2013 at 9:17

RobIII · Accepted Answer · 2013-11-25 08:57:45Z

5

Since CatID is an int, no, in this case you're not vulnerable to SQL injection. But the path you've chosen is a slippery slope and, someday, prone to SQL injection when refactoring or changing your code. It's better to get into the habit of using parameterized queries and sticking with it.

I can wholehartedly suggest you try Dapper (which is available as a Nuget package(; this will greatly simplify things and you won't have to change that much for it's benefits.

Your code will then become somthing like:

myConnection.Query<Customer>("SELECT * FROM TableXYZ WHERE CatID = @catid", new { catid = CatID });

answered Nov 25, 2013 at 8:57

RobIII

8,8414 gold badges46 silver badges99 bronze badges

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

4 Comments

Learning Over a year ago

I prefer Store procedure for better execution plan... I was just wondering if above example is vulnerable to sql injection taken into consideration input is being filter as int CatID = Request["CatID"]; I appreciate you suggestion i will look into Dapper

marc_s Over a year ago

But you could probably still pass something like 42 OR 1=1 and then basically get back all rows from TableXYZ... concatenating together your SQL is always a bad idea!

RobIII Over a year ago

@marc_s: I assume you're replying to KnowledgeSeeker because with Dapper, if that's what you're referring to, this will be handled (e.g. correctly parameterized) for you. Also, in the original question CatID is of type int so you wouldn't be able to pass 42 OR 1=1 but KnowledgeSeeker seems to be changing the game rules after the game has started with his reply above yours (where CatID is still of type int though, so again. he'd be safe as long as he's not using var or string) and making sure (when using var for example) that the parameter is parsed as int.

ta.speot.is Over a year ago

@KnowledgeSeeker SQL Server won't generate a better execution plan because you're using a stored procedure. It's not like "oh, a stored procedure ... I'd better do good this time."

user2604650 · Accepted Answer · 2013-11-25 08:55:13Z

4

Of course, it's not a good practice to format a query string directly. SqlParameter class is designed right for your purposes - to simplify query building and prevent SQL injections in full

answered Nov 25, 2013 at 8:55

user2604650

Collectives™ on Stack Overflow

SQL Injection vulnerability using int parameter

2 Answers 2

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related