-1

I am creating a basic registration page using ASP.NET websites and C# and am trying to link the logins to a database I have created in Visual Studio 2017 and am constantly getting the error -

'System.NullReferenceException: 'Object reference not set to an instance of an object.'

System.Data.Common.DbCommand.ExecuteScalar(...) returned null.

and cannot understand why, code below any help appreciated.

using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.UI;
using System.Web.UI.WebControls;
using System.Data.SqlClient;
using System.Configuration;
public partial class Registration : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        ValidationSettings.UnobtrusiveValidationMode = UnobtrusiveValidationMode.None;

        if (IsPostBack)
        {

            SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["RegistrationConnectionString"].ConnectionString);
            conn.Open();
            string checkuser = "SELECT * FROM [Table] WHERE UserName='" + TextBoxUN.Text + "'";
            SqlCommand com = new SqlCommand(checkuser, conn);
            int temp = Convert.ToInt32(com.ExecuteScalar().ToString());
            if (temp == 1)
            {
                Response.Write("User already exists, please enter a different username");
            }


            conn.Close();


        }
    }

    protected void Button1_Click(object sender, EventArgs e)
    {
        try
        {
            SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["RegistrationConnectionString"].ConnectionString);
            conn.Open();
            string insertQuery = "INSERT INTO Table (UserName,Email,Password,Country) values(@Uname ,@email , @password ,@country)";
            SqlCommand com = new SqlCommand(insertQuery, conn);
            com.Parameters.AddWithValue("@Uname" , TextBoxUN.Text);
            com.Parameters.AddWithValue("@email" , TextBoxEmail.Text);
            com.Parameters.AddWithValue("@password" , TextBoxPass.Text);
            com.Parameters.AddWithValue("@country" , DropDownListCountry.SelectedItem.ToString());

            com.ExecuteNonQuery();
            Response.Redirect("Manager.aspx");
            Response.Write("Registration Successful");

            conn.Close();
        }
        catch (Exception ex)
        {
            Response.Write("Error:" + ex.ToString());
        }
    }
}```

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • 3
    When using SQLServer, [avoid AddWithValue](https://www.dbdelta.com/addwithvalue-is-evil/) - other DBs don't necessarily care, but SQLS does – Caius Jard Jan 13 '22 at 17:10
  • 1
    `ExecuteScalar` is for reading a single value, you're selecting all columns from `Table`. What output are you expecting? I'm guessing you want something like `select count(1) from...` in the sql. And you should parameterize both queries – haldo Jan 13 '22 at 17:12
  • 3
    **Always use parameterized sql and avoid string concatenation** to add values to sql statements. This mitigates SQL Injection vulnerabilities and ensures values are passed to the statement correctly. See [How can I add user-supplied input to an SQL statement?](https://stackoverflow.com/q/35163361/1260204), and [Exploits of a Mom](https://xkcd.com/327/). You do this already in `Button1_Click` but not in `Page_Load`. – Igor Jan 13 '22 at 17:14
  • 1
    In addition to the SQL Injection vulnerability, you are storing passwords in plain text. Don't do that! [Secure Password Authentication Explained Simply](https://www.codeproject.com/Articles/54164/Secure-Password-Authentication-Explained-Simply); [Salted Password Hashing - Doing it Right](https://www.codeproject.com/Articles/704865/Salted-Password-Hashing-Doing-it-Right) – Richard Deeming Jan 13 '22 at 17:16
  • 1
    If you want to roll your own authentication system where you store the credentials then you should always hash the password and store that hash and never the actual password itself. Here are some acceptable hashing algorithms to use for passwords. [pbkdf2](https://en.wikipedia.org/wiki/PBKDF2), [scrypt](https://en.wikipedia.org/wiki/Scrypt), [bcrypt](https://en.wikipedia.org/wiki/Bcrypt) – Igor Jan 13 '22 at 17:18

1 Answers1

2

If you want to check if user exists, there's no need in ExecuteScalar() with value:

int temp = Convert.ToInt32(com.ExecuteScalar().ToString());

when user doesn't exist, com.ExecuteScalar() returns null and you have a problem.

Code

private static bool UserExists(string userName) {
  if (null == userName)
    return false;

  using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["RegistrationConnectionString"].ConnectionString)) {
    conn.Open();

    //DONE: parametrized query
    //DONE: 1 instead of * - we don't want all columns to be returned 
    string sql =
      @"select 1
          from [Table]
         where UserName = @userName";

    using (SqlCommand com = new SqlCommand(sql, conn)) {
      com.Parameters.Add("userName", SqlDbType.VarChar).Value = userName;

      // user exists if cursor is not empty (we have at least one record) 
      return com.ExecuteScalar() != null; 
    }
  }
}

protected void Page_Load(object sender, EventArgs e) {
  ValidationSettings.UnobtrusiveValidationMode = UnobtrusiveValidationMode.None;

  if (IsPostBack && UserExists(TextBoxUN.Text)) 
    Response.Write("User already exists, please enter a different username");  
}
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
  • Should there be a top 1 instead of just 1 if this table is quite large and it would return all the rows just to check if 1 existed? Or use count? – Nikki9696 Jan 13 '22 at 17:28
  • 2
    @Nikki9696: asuming that `Table` have no *duplicate* user names, we can get either a *single record* (which corresponds to given user name) or an *empty cursor*. In case of single record `ExecuteScalar` return 1st field of the 1st record - `1`, in case of empty cursor - `null`. I hope, that `Table` has an *index* on `UserName` (at least to ensure that values are *unique*), in this case it doesn't matter how long the table is – Dmitry Bychenko Jan 13 '22 at 17:34