none
How can I avoid sql injection in my code RRS feed

  • Question

  • I have the following function, it seems that it has sql in jection, what I have to add to the code to avoid it?

         public static Sql Execute(string command, List<SqlParameter> parameters = null)

              

                }

                return new Sql(sqlConnection, reader);

            }



    • Edited by Gulnar78 Monday, November 25, 2019 8:21 AM edit
    Wednesday, October 2, 2019 5:56 PM

All replies

  • Hello,

    Can you explain what your concern is with the current code in regards to SQL Injection?


    Please remember to mark the replies as answers if they help and unmarked them if they provide no help, this will help others who are looking for solutions to the same or similar problem. Contact via my Twitter (Karen Payne) or Facebook (Karen Payne) via my MSDN profile but will not answer coding question on either.

    NuGet BaseConnectionLibrary for database connections.

    StackOverFlow
    profile for Karen Payne on Stack Exchange

    Wednesday, October 2, 2019 7:04 PM
    Moderator
  • Hi Gulnar78,

    Thank you for posting here.

    For your question, what is the sql injection? Please provide more details.

    Best Regards,

    Wendy


    MSDN Community Support
    Please remember to click "Mark as Answer" the responses that resolved your issue, and to click "Unmark as Answer" if not. This can be beneficial to other community members reading this thread. If you have any compliments or complaints to MSDN Support, feel free to contact MSDNFSF@microsoft.com.

    Thursday, October 3, 2019 2:27 AM
    Moderator
  • Where did the parameters come from? How do you know a SQL-Injection attack happened?
    Thursday, October 3, 2019 5:47 AM
  • SQL injection occurs when the query you run is using input provided by the user. That is completely tied to the value in `command` that you posted. Post that code. If that query is just using the parameters you are adding via the `parameters` list then you don't have a SQL injection problem.

    //SQL injection possibility
    var command = "SELECT * FROM Table WHERE Column = '" + userInput + "'";
    
    //NO SQL injection issue
    var command = "SELECT * FROM Table WHERE Column = @value"
    
    cmd.Parameters.Add("@value", "");
    


    Michael Taylor http://www.michaeltaylorp3.net

    Thursday, October 3, 2019 2:34 PM
    Moderator
  • Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection'), In the line that is underlined.
    Friday, October 4, 2019 6:38 PM
  • It is reviewed by somebody and he told me that it contains sql injection in the undelined line in the function above
    Friday, October 4, 2019 6:39 PM
  • Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')
    Friday, October 4, 2019 6:40 PM
  • How can I apply it on my code above?
    Friday, October 4, 2019 6:42 PM
  • Again, you haven't actually provided us the actual command you're executing so we cannot possibly answer that question. Might point is that your code, as it is written, does not have a SQL injection problem. It is correct in that regard. What matters is that command parameter you're passing to it. That is where the attack can come. Since you didn't post that command we cannot tell you where it is going wrong.

    //This is a SQL injection attack - at the caller level
    ExecuteSqlCommand("SELECT * FROM Table WHERE Column = '" + someValueFromUser + "'", new List<SqlParameter>());
    
    //This does not have the issue - again, at the caller level
    var parameters = new List<SqlParameter>() {
       new SqlParameter("@value", someValueFromuser)
    };
    ExecuteSqlCommand("SELECT * FROM Table WHERE Column = @value", parameters);
    Again, this has absolutely nothing to do with the code you posted. Unto itself it does not have an SQL injection issues. It is completely dependent upon the command the caller is passing. To figure out whether you have an inject attack or not find all places you CALL this method. Every place you do so you need to rewrite the command you are passing to your method to make sure it is using parameters for any user-provided values.


    Michael Taylor http://www.michaeltaylorp3.net

    Friday, October 4, 2019 7:26 PM
    Moderator
  • Problem of this solution is that you don't know which type of command is passed through command parameter. It could be problem for some analyzers which can give notice there can be SQL injection in code. It is true because analyzer doesn't have context of whole data layer and call hierarchy. It could be false positive notice because all commands can be correct in method that uses ExecuteSqlCommand method. It is about programmers skills and practices if they will create SQL command which let to do SQL injection or not. I don't see useful in encapsulation SqlCommand into some SQL library or something similar. 
    Thursday, October 10, 2019 6:18 AM
  • Thank you for discussing.

       I have the following sql statement, Can you find where I have the sql injection?
       
       Sql sql = DBUtils.ExecuteSqlCommand("SELECT DISTINCT App.id ,App.Deposit, App.Ar, App.En, App.s FROM App, CV, Age WHERE [CV].Role = 9 and App.Id = CV.AppId AND Age.Id = @AgeId AND CV.UserId = Age.Id AND Deposit IS NOT NULL order by applications.Id desc",
                new List<SqlParameter> { new SqlParameter("@Age", Session["Patent:ID"]) });

      Sql sqldet = DBUtils.ExecuteSqlCommand("select InDoc.* ,Out.OutDate,OutgDoc.text as OutType,InDoc.text as InType from InDoc,Out,OutDoc where Out.OutType=OutDoc.Id AND Out.Deposit=@deposit AND OutDoc.FKInDocID NOT IN (select InDoc.id from In, InDoc where In.InType=InDoc.Id AND In.Deposit=@deposit) AND InDoc.Id=OutDoc.FKInDocID",
                        new List<SqlParameter> { new SqlParameter("@deposit", (int)sql.Reader["Deposit"]) });

    Sql sqlincome = DBUtils.ExecuteSqlCommand("select * from In  where  Deposit=@deposit AND InType=2 OR InType=61 OR InType= 62 OR InType= 63 ",
                                new List<SqlParameter> { new SqlParameter("@deposit", (int)sql.Reader["Deposit"]) });

       Sql sql = DBUtils.ExecuteSqlCommand("SELECT DISTINCT App.id ,App.Deposit, App.Ar, App.En, App.s FROM App, CV, Age WHERE [CV].UserRole = 8 and App.Id = CV.AppId AND Age.Id = @Age AND CV.UId = Age.Id AND Deposit IS NOT NULL order by app.Id desc",
                new List<SqlParameter> { new SqlParameter("@Age", Session["Patent:ID"]) });

                Sql sql = DBUtils.ExecuteSqlCommand("SELECT [NAr],[NEn],[Scope],[SAr],[SEn],[ENo],[SNo],[OpDate],[OrderType],[InDepasetN],[InDepositD],[InterAppN],[InterAppD],[St],[Code] FROM [App] where [ID] =" +lblAppId.Text);

                Sql sql = ExecuteSqlCommand("select r.TextAr AS country ,COUNT(r.TextAr) AS app_tot from (select distinct  CVs.Nation ,App.Deposit,lookup.TextAr from CVs,CV,App ,Grante ,lookup where Lookup.Id=CVs.Nationality AND CV.UserId=CVs.Id AND CV.AppId=App.id AND App.DepositNo =Granted.DepositNo  AND Applications.OrderType=1 AND GrantDate >= '"+year+"/1/1' and GrantDate < '"+ year + "/12/31  23:59:59.990') r GROUP BY TextAr");

                Sql sql = ExecuteSqlCommand("select r.TextAr AS country ,COUNT(r.TextAr) AS app_total from (select distinct  CVs.National ,App.Deposit,lookup.TextAr from CVs,CV,App ,Grante ,lookup where Lookup.Id=CVs.National AND CV.UId=CVs.Id AND CV.AppId=App.id AND App.Deposit =Granted.Deposit  AND App.OrderType=1 AND GrantDate >= '"+year+"/1/1' and GrantDate < '"+ year + "/12/31  23:59:59.990') r GROUP BY TextAr");

                sql = ExecuteSqlCommand("select r.TextAr AS country ,COUNT(r.TextAr) AS app_total from (select distinct  CVs.National ,App.Deposit,lookup.TextAr from CVs,CV,App ,Grante ,lookup where Lookup.Id=CVs.National AND CV.UId=CVs.Id AND CV.AppId=App.id AND App.Deposit =Grante.Deposit  AND App.OrderType=2 AND GrantDate >= '" + year + "/1/1' and GrantDate < '" + year + "/12/31  23:59:59.990') r GROUP BY TextAr");

                Sql sql = ExecuteSqlCommand("select LAtt from CVs,App,CV where  App.Id = CV.AppId AND App.Deposit = @deposit AND CVs.Id = CV.UId AND LAtt>= 9", parameters);

       Sql sql = ExecuteSqlCommand("select LFee.* ,Out.OutDate," +
                    " L_OutDoc.text as OutcomeType,L_InDoc.text as InType from LFee," +
                    "L_InDoc,Out,L_OutDoc where " +
                    "Out.OutType=L_OutDoc.Id AND Out.Deposit=@deposit" +
                    " AND L_OutDoc.FKLInDocID NOT IN (select Lookup_InDoc.id from In," +
                    " L_InDoc where In.InType=L_InDoc.Id AND In.Deposit=@deposit)" +
                    " AND L_InDoc.Id=L_OutDoc.FKLInDocID AND" +
                    " L_InDoc.FeeType=LFee.Id",
                new List<SqlParameter> { new SqlParameter("@deposit", deposit) });

    public static List<AppAttach> getAttachments(int appId) {
                List<AppAttach> attachments = new List<AppAttach>();
                Sql sql = ExecuteSqlCommand("SELECT * FROM [AppAttach],[Lookup] where[AppAttach].AppId =@appId AND[Lookup].Id =[AppAttach].AttachId  AND AttachId<>459 AND AttachId<>463 AND AttachId<>462",
                 new List<SqlParameter> { new SqlParameter("appId", appId) });
           
       
         public static List<string> getAttachm(int appId) {
                List<string> attachs = new List<string>();
                Sql sql = ExecuteSqlCommand("SELECT  InFile  FROM In where InType =2 OR  InType =61 OR InType =62 OR IneType=63  AND Deposit IN (SELECT Deposit FROM App where id=@appId)",
                 new List<SqlParameter> { new SqlParameter("appId", appId) });
     
     

    Tuesday, October 15, 2019 8:37 AM
  • Again, you haven't actually provided us the actual command you're executing so we cannot possibly answer that question. Might point is that your code, as it is written, does not have a SQL injection problem. It is correct in that regard. What matters is that command parameter you're passing to it. That is where the attack can come. Since you didn't post that command we cannot tell you where it is going wrong.

    //This is a SQL injection attack - at the caller level
    ExecuteSqlCommand("SELECT * FROM Table WHERE Column = '" + someValueFromUser + "'", new List<SqlParameter>());
    
    //This does not have the issue - again, at the caller level
    var parameters = new List<SqlParameter>() {
       new SqlParameter("@value", someValueFromuser)
    };
    ExecuteSqlCommand("SELECT * FROM Table WHERE Column = @value", parameters);
    Again, this has absolutely nothing to do with the code you posted. Unto itself it does not have an SQL injection issues. It is completely dependent upon the command the caller is passing. To figure out whether you have an inject attack or not find all places you CALL this method. Every place you do so you need to rewrite the command you are passing to your method to make sure it is using parameters for any user-provided values.


    Michael Taylor http://www.michaeltaylorp3.net

    Thank you for discussing.

       I have the following sql statement, Can you find where I have the sql injection?
       
       Sql sql = DBUtils.ExecuteSqlCommand("SELECT DISTINCT App.id ,App.Deposit, App.Ar, App.En, App.s FROM App, CV, Age WHERE [CV].Role = 9 and App.Id = CV.AppId AND Age.Id = @AgeId AND CV.UserId = Age.Id AND Deposit IS NOT NULL order by applications.Id desc",
                new List<SqlParameter> { new SqlParameter("@Age", Session["Patent:ID"]) });

      Sql sqldet = DBUtils.ExecuteSqlCommand("select InDoc.* ,Out.OutDate,OutgDoc.text as OutType,InDoc.text as InType from InDoc,Out,OutDoc where Out.OutType=OutDoc.Id AND Out.Deposit=@deposit AND OutDoc.FKInDocID NOT IN (select InDoc.id from In, InDoc where In.InType=InDoc.Id AND In.Deposit=@deposit) AND InDoc.Id=OutDoc.FKInDocID",
                        new List<SqlParameter> { new SqlParameter("@deposit", (int)sql.Reader["Deposit"]) });

    Sql sqlincome = DBUtils.ExecuteSqlCommand("select * from In  where  Deposit=@deposit AND InType=2 OR InType=61 OR InType= 62 OR InType= 63 ",
                                new List<SqlParameter> { new SqlParameter("@deposit", (int)sql.Reader["Deposit"]) });

       Sql sql = DBUtils.ExecuteSqlCommand("SELECT DISTINCT App.id ,App.Deposit, App.Ar, App.En, App.s FROM App, CV, Age WHERE [CV].UserRole = 8 and App.Id = CV.AppId AND Age.Id = @Age AND CV.UId = Age.Id AND Deposit IS NOT NULL order by app.Id desc",
                new List<SqlParameter> { new SqlParameter("@Age", Session["Patent:ID"]) });

                Sql sql = DBUtils.ExecuteSqlCommand("SELECT [NAr],[NEn],[Scope],[SAr],[SEn],[ENo],[SNo],[OpDate],[OrderType],[InDepasetN],[InDepositD],[InterAppN],[InterAppD],[St],[Code] FROM [App] where [ID] =" +lblAppId.Text);

                Sql sql = ExecuteSqlCommand("select r.TextAr AS country ,COUNT(r.TextAr) AS app_tot from (select distinct  CVs.Nation ,App.Deposit,lookup.TextAr from CVs,CV,App ,Grante ,lookup where Lookup.Id=CVs.Nationality AND CV.UserId=CVs.Id AND CV.AppId=App.id AND App.DepositNo =Granted.DepositNo  AND Applications.OrderType=1 AND GrantDate >= '"+year+"/1/1' and GrantDate < '"+ year + "/12/31  23:59:59.990') r GROUP BY TextAr");

                Sql sql = ExecuteSqlCommand("select r.TextAr AS country ,COUNT(r.TextAr) AS app_total from (select distinct  CVs.National ,App.Deposit,lookup.TextAr from CVs,CV,App ,Grante ,lookup where Lookup.Id=CVs.National AND CV.UId=CVs.Id AND CV.AppId=App.id AND App.Deposit =Granted.Deposit  AND App.OrderType=1 AND GrantDate >= '"+year+"/1/1' and GrantDate < '"+ year + "/12/31  23:59:59.990') r GROUP BY TextAr");

                sql = ExecuteSqlCommand("select r.TextAr AS country ,COUNT(r.TextAr) AS app_total from (select distinct  CVs.National ,App.Deposit,lookup.TextAr from CVs,CV,App ,Grante ,lookup where Lookup.Id=CVs.National AND CV.UId=CVs.Id AND CV.AppId=App.id AND App.Deposit =Grante.Deposit  AND App.OrderType=2 AND GrantDate >= '" + year + "/1/1' and GrantDate < '" + year + "/12/31  23:59:59.990') r GROUP BY TextAr");

                Sql sql = ExecuteSqlCommand("select LAtt from CVs,App,CV where  App.Id = CV.AppId AND App.Deposit = @deposit AND CVs.Id = CV.UId AND LAtt>= 9", parameters);

       Sql sql = ExecuteSqlCommand("select LFee.* ,Out.OutDate," +
                    " L_OutDoc.text as OutcomeType,L_InDoc.text as InType from LFee," +
                    "L_InDoc,Out,L_OutDoc where " +
                    "Out.OutType=L_OutDoc.Id AND Out.Deposit=@deposit" +
                    " AND L_OutDoc.FKLInDocID NOT IN (select Lookup_InDoc.id from In," +
                    " L_InDoc where In.InType=L_InDoc.Id AND In.Deposit=@deposit)" +
                    " AND L_InDoc.Id=L_OutDoc.FKLInDocID AND" +
                    " L_InDoc.FeeType=LFee.Id",
                new List<SqlParameter> { new SqlParameter("@deposit", deposit) });

    public static List<AppAttach> getAttachments(int appId) {
                List<AppAttach> attachments = new List<AppAttach>();
                Sql sql = ExecuteSqlCommand("SELECT * FROM [AppAttach],[Lookup] where[AppAttach].AppId =@appId AND[Lookup].Id =[AppAttach].AttachId  AND AttachId<>459 AND AttachId<>463 AND AttachId<>462",
                 new List<SqlParameter> { new SqlParameter("appId", appId) });
           
       
         public static List<string> getAttachm(int appId) {
                List<string> attachs = new List<string>();
                Sql sql = ExecuteSqlCommand("SELECT  InFile  FROM In where InType =2 OR  InType =61 OR InType =62 OR IneType=63  AND Deposit IN (SELECT Deposit FROM App where id=@appId)",
                 new List<SqlParameter> { new SqlParameter("appId", appId) });
    Tuesday, October 15, 2019 11:12 AM
  • All the queries from 

    Sql sql = DBUtils.ExecuteSqlCommand("SELECT [NAr],[NEn],[Scope],[SAr],[SEn],[ENo],[SNo],[OpDate],[OrderType],[InDepasetN],[InDepositD],[InterAppN],[InterAppD],[St],[Code] FROM [App] where [ID] =" +lblAppId.Text);

    down are vulnerable. It is very easy to tell if you're vulnerable. If anywhere in your query you use string concatenation, then most likely you're vulnerable. Every place you are current using string concat to put a value directly into the query, replace with a parameter instead. You queries should consist of a single string literal with any replaceable values referenced using parameters. 


    Michael Taylor http://www.michaeltaylorp3.net

    Tuesday, October 15, 2019 2:34 PM
    Moderator