1

I'm trying to run this register form and whenever I try to run it I get an error saying that I have a mistake when using the INSERT INTO command.

Here is the error log: https://i.stack.imgur.com/nuu2B.png

And here is the full code:

protected void Page_Load(object sender, EventArgs e)
{
    string username = Request.Form["username"];
    string password = Request.Form["password"];
    string email = Request.Form["email"];

    OleDbConnection dbCon = new OleDbConnection();
    OleDbCommand dbCmd = new OleDbCommand();

    String Path = Server.MapPath(@"../App_Data/XXX.mdb;");
    dbCon.ConnectionString = "Provider=Microsoft.Jet.OLEDB.4.0;Data source=" + Path;
    dbCmd.Connection = dbCon;

    OleDbDataAdapter dataA = new OleDbDataAdapter(dbCmd);
    dbCmd.CommandText = String.Format("SELECT * FROM Members where username = '{0}';", username);
    DataTable dataT = new DataTable();
    dataA.Fill(dataT);

    if (dataT.Rows.Count == 0)
    {
        //dbCmd.CommandText = String.Format("INSERT INTO Members (username, password, email) VALUES ('{0}','{1}','{2}');", username, password, email);
        dbCmd.CommandText = String.Format("INSERT INTO Members (username, password, email) VALUES ('" + username + "','" + password + "','" + email + "')");
        dbCon.Open();
        dbCmd.ExecuteNonQuery();
        dbCon.Close();
    }
    else
    {
        Response.Write("That username is alredy taken");
        Response.Redirect("register.aspx");
    }
}

I want it to execute the command properly and add it to the database. As you can see, I tried using 2 methods of entering data into the database

dbCmd.CommandText = String.Format("INSERT INTO Members (username, password, email) VALUES ('{0}','{1}','{2}');", username, password, email);

And

dbCmd.CommandText = String.Format("INSERT INTO Members (username, password, email) VALUES ('" + username + "','" + password + "','" + email + "')");

None of them worked.

Can anyone help me solve this problem?

Thank you :)

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Shaked Dahan
  • 402
  • 5
  • 22
  • 1
    Fyi, your query is super vulnerable to SQL Injection – Nick Bailey Mar 10 '15 at 20:52
  • How did they not work? Did you get an exception? Are they not in the database? You should also look into using parameters instead of creating a string. – D Stanley Mar 10 '15 at 20:52
  • Also note that your `response.Write` is useless since you immediately redirect to `register.aspx` – D Stanley Mar 10 '15 at 20:53
  • Do it in try catch block and look at full stack – Giorgi Nakeuri Mar 10 '15 at 20:53
  • if you user name is O'Reilly the code will fail - use parameters – Hugh Jones Mar 10 '15 at 21:15
  • what is the effect of an `@` symbol in an email address? – Hugh Jones Mar 10 '15 at 21:20
  • read this article in full http://www.csharp-station.com/Tutorial/AdoDotNet/lesson06 – Hugh Jones Mar 10 '15 at 21:25
  • `password` is a reserved word. See [answer here](http://stackoverflow.com/a/9852715/77335) – HansUp Mar 10 '15 at 21:42
  • @NickBailey my qury is not vulnerable to sql injesction since i used javascriped to check the input, the input may only containt A-Z, a-z or 1-9. same goes for password and for the email there is an exectption to accept only '@'. anyway, thank you for the help guys, if i wont be able to solve this i guess ill have to ask my teacher what is wrong. thank you again! :) – Shaked Dahan Mar 11 '15 at 13:29
  • 1
    @Shaked - why not try the suggestion here to create parameters for your query? Your teacher will be impressed at your diligence. – Hugh Jones Mar 12 '15 at 08:34
  • Seems like you have come up with a new way to stop sql injection that _no one else has thought of_. Anyway @HansUp has your answer. – Nick.Mc Feb 18 '16 at 00:58

2 Answers2

1

The problem is that Email addresses contain the @ symbol, which is interpreted as a sql parameter.

The solution is to use parameters properly

Hugh Jones
  • 2,706
  • 19
  • 30
0

TLDR; the code is bad, open to all sorts of stuff. db conn info in plain site should not be. you need a parameterized stored procedure to do this safely. Sanitize the data on the client side, and write your code to handle special symbols.Set the form collected values to SQL input params, call your sproc passing them in, so something based on the return value.

(the longer explanation) 1. Put your connection string information into a web.config file 2. Enforce some client side validation 3. Create a single stored procedure that will update or insert based on if the name already exists 4. Run your code to login passing into the SPROC the data you collected from your form as the input parameters to the sproc.

i am missing a lot of pseudo code etc. If you were curious about the concept that is it, If you are looking for the actual code I can help you out another time.

SSISPissesMeOff
  • 412
  • 1
  • 5
  • 15