Member Avatar for Nemo_NIIT

Hi

I am using web application.

on my login button i have written this code

protected void btnlogin_Click(object sender, EventArgs e)
    {
        //sql connection
        SqlConnection cnn = new SqlConnection();
        cnn.ConnectionString = "Server=HOME-PC;Database=kandivalideal;Trusted_Connection=True;";
        cnn.Open();
        SqlCommand cmd = new SqlCommand("select mobileno, password from register where mobileno='"+txtmoblie.Text +"'and password='"+txtpassword.Text+"'",cnn);
        cmd.Connection = cnn;
        SqlDataReader dr = cmd.ExecuteReader();

        if (txtmoblie.Text != "mobileno" && txtpassword.Text != "password")
        {
            Response.Write(@"<script language='javascript'>alert('Invalid Username and Password')</script>");
        }
        else
        {
            Response.Redirect("signup.aspx");
        }
        cnn.Close();
       


    }

I think my If statement is giving me error it is not getting moblie no and password it is giving error by say invalid username and password

  • 6 Contributors
  • 10 Replies
  • 469 Views
  • 5 Days Discussion Span
  • Latest Post Latest Post by esmaeel2001

Dani AI

Generated

The login always failed because the code never used the query result and instead tested the textbox values against literal placeholders (so entering a real mobile/password still fails). and were correct to point out that the database result must be checked, and correctly warned about SQL injection from concatenating inputs into the SQL string. The immediate fixes are: stop comparing to placeholder text, stop embedding raw textbox values into SQL, and only decide success after verifying the database record.

Recommended, minimal changes (high-level):

  • Use parameterized queries and dispose connections/commands with using blocks (prevents SQL injection and leaks).
  • Store passwords as salted hashes (never plain text). Return only the stored hash+salt for a mobile number and verify the supplied password against that hash.
  • Check whether the query returned a row (presence) and then verify the password; treat missing row and wrong password the same way for timing/UX consistency.
  • Add trimming/normalization for the mobile string, proper error logging, and a retry/lockout policy to limit brute force attempts.

Example verification flow (illustrative, not already posted in thread):

const string sql = "SELECT PasswordHash, PasswordSalt FROM register WHERE mobileno = @mobile";
using (var conn = new SqlConnection(connString))
using (var cmd = new SqlCommand(sql, conn))
{
    cmd.Parameters.Add("@mobile", SqlDbType.NVarChar, 50).Value = txtMobile.Text.Trim();
    conn.Open();
    using (var rdr = cmd.ExecuteReader())
    {
        if (rdr.Read())
        {
            var storedHash = Convert.FromBase64String(rdr.GetString(0));
            var storedSalt = Convert.FromBase64String(rdr.GetString(1));
            if (VerifyPassword(txtPassword.Text, storedSalt, storedHash))
            {
                // successful login -> redirect to secure page
            }
            else
            {
                // invalid credentials
            }
        }
        else
        {
            // invalid credentials
        }
    }
}

bool VerifyPassword(string password, byte[] salt, byte[] expected)
{
    using (var pbkdf2 = new Rfc2898DeriveBytes(password, salt, 10000))
        return pbkdf2.GetBytes(expected.Length).SequenceEqual(expected);
}

For longer-term maintenance, prefer the built-in ASP.NET authentication (Identity/Owin) rather than rolling custom login code; combine parameterization, hashing, and proper error/logging for robust security.

Recommended Answers

All 10 Replies

Member Avatar for hericles

You're not using the data extracted in your datareader at all. You are setting up the connection and calling the executeReader method then leaping into the If statement. It is generating the error because you haven't entered "mobileno" and "password" into the two textboxes.

You need to check if the datareader has one row. If it does your SQL statement worked and the user can log in.

Member Avatar for Nemo_NIIT

Can you give me some example for this please


You're not using the data extracted in your datareader at all. You are setting up the connection and calling the executeReader method then leaping into the If statement. It is generating the error because you haven't entered "mobileno" and "password" into the two textboxes.

You need to check if the datareader has one row. If it does your SQL statement worked and the user can log in.

Member Avatar for Nemo_NIIT 
protected void btnlogin_Click(object sender, EventArgs e)
    {
        //sql connection
        SqlConnection cnn = new SqlConnection();
        cnn.ConnectionString = "Server=HOME-PC;Database=kandivalideal;Trusted_Connection=True;";
        cnn.Open();
        SqlCommand cmd = new SqlCommand("select mobileno, password from register where mobileno='"+txtmoblie.Text +"'and password='"+txtpassword.Text+"'",cnn);
        cmd.Connection = cnn;
        SqlDataReader dr = cmd.ExecuteReader();

        if (dr.Read())
        {
            Response.Redirect("http://www.google.com");
        }
        else
        {
            Response.Write(@"<script language='javascript'>alert('Invalid Username and Password')</script>");
            
        }
        cnn.Close();
       


    }
Edited by kvprajapati because: Correct [code] tags.
Member Avatar for kamilacbe

You have assign the value read from datareader to a temp string and then you can check with your database string as you have done.:-) ( the prob you have to sort is datareader value.)

Member Avatar for ChrisHunter

if you do something like the following example the COUNT() function will count the amount of rows within the database that match the WERE clause and return that number. Then you check the number is 0 or not and grant or deny access accordingly.

"SELECT COUNT(*) my MyTable WHERE Username = ' + textbox1.text + ' AND Password = ' + textbox2.text + '";
Member Avatar for esmaeel2001

samply, you execute a SQL statement that select all the ID's from your table that meet the condition:
WHERE Username = ' + textbox1.text + ' AND Password = ' + textbox2.text
if the user is exist and the password is correct you should get one row in the datareader Else the datareder.Read() method will return null.

then ... after you execute the command check: if (!Datareader.Read()) that mean No rows, thus, no users or password not valid!

Member Avatar for ChrisHunter

if you do something like the following example

Sorry i missed the FROM clause out:

"SELECT COUNT(*) FROM MyTable WHERE Username = ' + textbox1.text + ' AND Password = ' + textbox2.text + '";
Edited by Dani because: Formatting fixed
Member Avatar for hericles

Everyone's code here is ok from a login point but they all include the textbox text directly in the SQL statement. That is bad, it allows for SQL injection attacks. In the interests of becoming a better code get used to using the parameters of the command object to encapsulate the inputs

MySqlCommand cmd = new MySqlCommand();
cmd.Parameters.Add("?userName", MySqlDataType.Varchar);
cmd.Parameters["?userName"].Value = Textbox1.Text;

Repeat that for the password textbox. It prevents users from slipping in SQL code that can subvert your security.

Member Avatar for esmaeel2001

hericles! good advise! thx alot

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.