CODEWITHSUNDEEP
c#sql-serverasp.net-mvcasp.net-mvc-4
Logan Jones
Logan Jones

Reputation: 77

MVC 4 SQL string injection Security

Hi I am fairly new to MVc and C# i was wondering if i could use the model security to prevent SQL injections? I created a model that contains variables that we are recieving from client input then forming a SQL statement from them. I was wondering if the built in security in MVC would be enough to prevent SQL injections? Please have a look at the code any suggestions are greatly appreciated.

Model

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;

namespace DataBaseTest.Models
{
   public class BedroomModel
   {
    public string YrBlt1 { get; set; }
    public string YrBlt2 { get; set; }
    public string TotLivArea1 { get; set; }
    public string TotLivArea2 { get; set; }
    public string LotArea1 { get; set; }
    public string LotArea2 { get; set; }
    public string Bedrooms { get; set; }
    public string SalePrice1 { get; set; }
    public string SalePrice2 { get; set; }
    public string SaleDate { get; set; }
    public string AssesVal1 { get; set; }
    public string AssesVal2 { get; set; }
    public string Style { get; set; }
    public string ArchStyle { get; set; }
    public string TaxUnit { get; set; }

    // Criteria added during SQL Queries
    public string YearBuilt { get; set; }
    public string LivingArea{ get; set; }
    public string LotArea { get; set; }
    public string SalePriceA { get; set; }
    public string SaleDateA { get; set; }
    public string AssesVal { get; set; }
    public string StyleA { get; set; }
    public string ArchStyleA { get; set; }
    public string ParcelId { get; set; }
    public string QuickRefId { get; set; }
    public string TaxunitA { get; set; }
    public string Address { get; set; }
    public string ValCode { get; set; }
    public string BedroomA { get; set; }

Then our SQL

  [HttpPost]
    public ActionResult Index(DataBaseTest.Models.BedroomModel user,DataTable dtFindResults)
    {
        StringBuilder sbSQL = new StringBuilder();
        //// define a list of CustomerModel objects
        DataSet tempDS = new DataSet();

        //string xSQL = "SELECT PropertyAddress,PropertyTypeDesc,PropertyID FROM KDOR_vwPropertyGeneral ORDER BY PropertyAddress";
        System.Data.SqlClient.SqlDataAdapter DbCmd = new System.Data.SqlClient.SqlDataAdapter();
        string sqlWhereCont = " WHERE ";
        sbSQL.Append("SELECT ");
        //sbSQL.Append(SessionHandler.AddressPointsPointsIDColumn + " AS PointsID,");
        sbSQL.Append("pg.PropertyNumberSearch,");
        sbSQL.Append("pg.QuickRefID,");
        sbSQL.Append("pg.PropertyAddress,");
        sbSQL.Append("crb.fmsstyle,");
        sbSQL.Append("srb.farchstyle,");
        sbSQL.Append("pt.TransferValidityCode,");
        sbSQL.Append("pg.TaxingUnitGroupCode,");
        sbSQL.Append("pt.Price,");
        sbSQL.Append("pt.SaleDate,");
        sbSQL.Append("crb.fyrblt,");
        sbSQL.Append("crb.vResBldgDep_tla_value,");
        sbSQL.Append("lm.facres,");
        sbSQL.Append("crb.frmbed");
        sbSQL.Append(" FROM KDOR_vwPropertyGeneral pg ");
        sbSQL.Append(" Left Join cama_ResBldg crb ON pg.PropertyID = crb.PropertyID And pg.AdHocTaxYear = crb.AdHocTaxYear ");
        sbSQL.Append(" Left Join sales_ResBldg  srb ON pg.PropertyID = srb.PropertyID");
        sbSQL.Append(" Left Join KDOR_vwPropertyTransfer pt On pg.PropertyID = pt.PropertyID");
        sbSQL.Append(" Left Join cama_LandMkt lm ON pg.PropertyID = lm.PropertyID And pg.AdHocTaxYear = lm.AdHocTaxYear");
        if (!string.IsNullOrEmpty(user.YrBlt1)||!string.IsNullOrEmpty(user.YrBlt2))
        {
            //sbSQL.Append(sqlWhereCont +"PropertyAddress = '" + user.Address + "'");
            //sqlWhereCont = "AND ";
            sbSQL.Append(sqlWhereCont +"crb.fyrblt >="+ user.YrBlt1+ " And crb.fyrblt <=  " + user.YrBlt2 );
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.TotLivArea1) || !string.IsNullOrEmpty(user.TotLivArea2))
        {
            //sbSQL.Append(sqlWhereCont +"PropertyAddress = '" + user.Address + "'");
            //sqlWhereCont = "AND ";
            sbSQL.Append(sqlWhereCont + "crb.vResBldgDep_tla_value >=" + user.TotLivArea1 + " And crb.vResBldgDep_tla_value <=  " + user.TotLivArea2 );
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.LotArea1) || !string.IsNullOrEmpty(user.LotArea2))
        {
            //sbSQL.Append(sqlWhereCont +"PropertyAddress = '" + user.Address + "'");
            //sqlWhereCont = "AND ";
            sbSQL.Append(sqlWhereCont + "lm.facres >=" + user.LotArea1 + " And lm.facres <= " + user.LotArea2 );
            sqlWhereCont = "AND ";
        }

        if (!string.IsNullOrEmpty(user.Bedrooms))
        {
            sbSQL.Append(sqlWhereCont + "crb.frmbed = '" + user.Bedrooms + "'");
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.SalePrice1) || !string.IsNullOrEmpty(user.SalePrice2))
        {
            //sbSQL.Append(sqlWhereCont +"PropertyAddress = '" + user.Address + "'");
            //sqlWhereCont = "AND ";
            sbSQL.Append(sqlWhereCont + "pt.Price >=" + user.SalePrice1 + " And pt.Price <=  " + user.SalePrice2 );
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.SaleDate))
        {
            sbSQL.Append(sqlWhereCont + "pt.SaleDate = '" + user.SaleDate + "'");
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.AssesVal1) || !string.IsNullOrEmpty(user.AssesVal2))
        {
            //sbSQL.Append(sqlWhereCont +"PropertyAddress = '" + user.Address + "'");
            //sqlWhereCont = "AND ";
            sbSQL.Append(sqlWhereCont + "crb.vResBldgDep_tla_value >=" + user.AssesVal1 + " And crb.vResBldgDep_tla_value <= " + user.AssesVal2 );
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.Style))
        {
            sbSQL.Append(sqlWhereCont + "crb.fmsstyle = '" + user.Style + "'");
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.ArchStyle))
        {
            sbSQL.Append(sqlWhereCont + "srb.farchstyle = '" + user.ArchStyle + "'");
            sqlWhereCont = "AND ";
        }
        if (!string.IsNullOrEmpty(user.TaxUnit))
        {
            sbSQL.Append(sqlWhereCont + "pg.TaxingUnitGroupCode = '" + user.TaxUnit + "'");
            sqlWhereCont = "AND ";
        }
        sbSQL.Append(" ORDER BY ");
        sbSQL.Append(" pg.QuickRefID ");


        //// populate a list of CustomerModel objects from database
        string MyConnectionString = ConfigurationManager.ConnectionStrings["WLConnection"].ConnectionString;
        System.Data.SqlClient.SqlConnection cnn = new System.Data.SqlClient.SqlConnection(MyConnectionString);
        System.Data.SqlClient.SqlCommand cmd = new System.Data.SqlClient.SqlCommand(sbSQL.ToString(), cnn);
        cmd.CommandTimeout = 30000;
        DbCmd.SelectCommand = cmd;
        DbCmd.Fill(tempDS, "ResultSet");
        DataTable resultSet = tempDS.Tables["ResultSet"];
        var vm = new List<BedroomModel>();
       foreach (DataRow dr in tempDS.Tables[0].Rows)
        {
            vm.Add(new BedroomModel 
            {
                BedroomA = dr.ItemArray[12].ToString(),
                YearBuilt = dr.ItemArray[9].ToString(),
                LivingArea = dr.ItemArray[7].ToString(),
                LotArea = dr.ItemArray[3].ToString(),
                SaleDateA = dr.ItemArray[8].ToString(),
                SalePriceA = dr.ItemArray[10].ToString(),
                AssesVal = dr.ItemArray[5].ToString(),
                StyleA = dr.ItemArray[3].ToString(),
                ArchStyleA = dr.ItemArray[4].ToString(),
                ParcelId = dr.ItemArray[0].ToString(),
                QuickRefId = dr.ItemArray[1].ToString(),
                TaxunitA = dr.ItemArray[6].ToString(),
                Address = dr.ItemArray[2].ToString(),
                ValCode = dr.ItemArray[5].ToString(),


             });
          }
        //DbCmd.Fill(dtFindResults);
        //var x = dtFindResults.Rows.Count;
        cnn.Close();
       return View("Result",vm);
        //// return the list of CustomerModel objects to our View
        //return View("Result", resultSet);
        //return View(ViewBag.data);
    }

Upvotes: 0

Views: 986

Answers (1)

Tim M.
Tim M.

Reputation: 54368

Parameterized queries are a must. However, that doesn't prevent you from being able to build dynamic queries, you just need to approach it differently.

It's fine to make decisions based on user input; it's not fine to concatenate those values into a query.

A quick example:

using( IDbCommand cmd = GetCommand() )
{
    string lotSize = "12345";
    bool includeLotSize = !string.IsNullOrWhiteSpace( lotSize );

    var sb = new StringBuilder();
    sb.AppendLine( "SELECT Col1, Col2 FROM dbo.Foo" );

    // you might also vary the columns returned based on what the user asked for

    if( includeLotSize )
    {
        sb.AppendLine( "WHERE LotSize = @LotSize" );

        // The query will expect the lot size, so add a parameter here to pass 
        // the lot size value.
        cmd.Parameters.Add( new SqlParameter( "LotSize", lotSize ) );
    }
}

Note that many of your string properties look like they could be a more specific type (int, float, an int pointing to a database lookup, etc). This does not prevent SQL injection but it can be used for validation (as well as making your view models cleaner).

Also note that there are many different ways to connect to a database in .Net, but be sure that you are disposing of your resources properly (note the using statement I've added).

Upvotes: 5

Related Questions