2

I'm working on a school project to create a registration system. My chosen means of a DB was to use T-SQL as it's something I'm already familiar.

I'm using the below code to query a DB

    public void button3_Click(object sender, EventArgs e)
    {
        string StudentID = (textBox1.Text);
        string queryDOB = "SELECT DOB,LastName,Degree FROM Student Where StudentID = " + StudentID;

        SqlConnection con = new SqlConnection(Properties.Settings.Default.SRSConnectionString);
        con.Open();

        SqlCommand DOB = new SqlCommand(queryDOB, con);

        SqlDataReader stidreader = DOB.ExecuteReader();

        if (stidreader.Read())
        {
            textBox2.Text = stidreader["DOB"].ToString();
            textBox3.Text = stidreader["LastName"].ToString();
            textBox5.Text = stidreader["Degree"].ToString();
        }
        else
        {
            MessageBox.Show("Your ID is not recognised. Please try again.");
            textBox1.Clear();
            textBox2.Clear();
            textBox3.Clear();
            textBox5.Clear();
        }

        con.Close();
    }

    private void textBox5_TextChanged(object sender, EventArgs e)
    {
        string Degree = textBox5.Text;
        string degsql = "SELECT ModName FROM Module Where Degree = " + Degree;
        SqlConnection con = new SqlConnection(Properties.Settings.Default.SRSConnectionString);
        DataSet dsm = new DataSet();
        SqlDataAdapter connect = new SqlDataAdapter(degsql, con);

        con.Open();

        connect.Fill(dsm);
        this.listBox2.DataSource = dsm.Tables[0];
        this.listBox2.DisplayMember = "ModName";
}

The first section for the button click works perfectly but when I run the query in the next event (textbox change) the query returns an error message that an invalid column is found. Well, the column found is the variable that it should be using for the Query, not the column itself which is very confusing.

I know there can be issues with something called MARS but the DB seems to be the right version.

Can anyone shed any light?

Appreciate it,

Jamie

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • 4
    Eek, horrible SQL injection issue here! What happens if someone enters this in your text box: `5; DROP TABLE Student`? – DavidG Dec 02 '14 at 10:26

1 Answers1

3

You are missing single quotes around your variable name. Change your code to this to fix the SQL:

string degsql = "SELECT ModName FROM Module Where Degree = '" + Degree + "'";

Or this looks slightly better:

string degsql = string.Format(
    "SELECT ModName FROM Module Where Degree = '{0}'", 
    Degree);

However, you should NOT do it this way as it exposes you to SQL injection attacks. Read this question to understand what that means: How do parameterized queries help against SQL injection?

Here is some code to do it properly and avoid those nasty security issues:

private void textBox5_TextChanged(object sender, EventArgs e)
{
    string Degree = textBox5.Text;
    string degsql = "SELECT ModName FROM Module Where Degree = @Degree";

    DataSet dsm = new DataSet();

    using(var con = new SqlConnection(Properties.Settings.Default.SRSConnectionString))
    {
        SqlCommand cmd = new SqlCommand(degsql, con);
        // use the appropriate SqlDbType:
        var parameter = new SqlParameter("@Degree", SqlDbType.Varchar);
        parameter.Value = Degree;
        cmd.Parameters.Add(parameter);

        SqlDataAdapter connect = new SqlDataAdapter(cmd);
        connect.Fill(dsm);
    }

    this.listBox2.DataSource = dsm.Tables[0];
    this.listBox2.DisplayMember = "ModName";
Community
  • 1
  • 1
DavidG
  • 113,891
  • 12
  • 217
  • 223
  • Thanks for all the help! I was certainly going down the wrong path and wondered if i was running into problems with connections etc. I knew the script was susceptible for Injection but wanted to check it was working before attempting parametrization. I tried the above script but still recieve an error, a different one though. I think it's because the values in my table are not var (E.g. engineering) and i get this result The SqlParameterCollection only accepts non-null SqlParameter type objects, not String objects. I have tried to convert but i'm a little stuck again! – Jamie Jardine Dec 02 '14 at 13:50
  • It seems there was a slight error in the code, probably somewhere between me posting and @TimSchmelter making the edit. I've fixed it for you. – DavidG Dec 02 '14 at 13:57
  • Thanks David/Tim - It's really helpful to have the explanation as well as the fixed code. – Jamie Jardine Dec 02 '14 at 14:51