Prompting user for connection parameters to SQL Server

I try to avoid keeping passwords etc. in memory or in plain text anywhere. But I am on a huge time crunch and this will only be used internally this week then probably won't get touched again. I just want to make sure this is secure and if not, what the risk is.

I'm mostly concerned about the password sitting in memory. I'm not sanitizing data but for this particular case it isn't needed.

My specific questions are:

-
From a security perspective, would this code be acceptable in a SQL Importer?

-
Besides data sanitation, is there a best practice somewhere that I'm missing in this?

```
Console.WriteLine("Enter the Server Name. Ex: SQLMASTER"); //Gets the server to connect to, an IP address is also acceptable.
ServerName = Console.ReadLine();
Console.WriteLine("Enter the Database. Ex: Accounts"); //The actual database to use.
DatabaseName = Console.ReadLine();
Console.WriteLine("Use Windows authentication? Y/N"); //you will usually use this.
YesNo = Console.ReadLine();

if (YesNo.ToLower() != "y")
{
Console.WriteLine("Enter your Username (Domain name may be required). Ex: MyName");
UserName = Console.ReadLine();
Console.WriteLine("Enter your Password. Ex: ****");
Password = ReadPassword();

//Builds the connection string with the data we collected above. We're going to send this to the SQL Connection. (Username and Password)
ConnectionString = "Data Source = " + ServerName + "; Initial Catalog = " + DatabaseName + "; User ID = " + UserName + "; Password = " + Password;

Password = ""; //We do this to clear the password from memory
UserName = ""; //Clearing username from memory
}
else
{
//Builds the connection string with the data we collected above. We're going to send this to the SQL Connection. (Windows Authentication)
ConnectionString = "Data Source = " + ServerName + "; Initial Catalog = " + DatabaseName + "; Integrated Security=SSPI"; //User ID = " + UserName + "; Password = " + Password;
}

//Create a new connec

It doesn't matter what you do with this code, the ConnectionString can always be accessed in the SqlConnection.

As a test, here's what you should do:

Launch your application outside of Visual Studio. Take a new instance of Visual Studio and go to 'Debug' -> 'Attach to Process...' and then locate the process for your programme.

Once you have done this, run your programme until the SQL point, then hit the 'Pause debugging' button in Visual Studio.

Once you have done this, go to the Diagnostic Tools and take a memory snapshot. While it's paused you can view the memory dump of the snapshot.

You should see a record appear in the 'Memory Usage' tab of the diagnostic tools, click the 'Objects (Diff)' link.

A new tab will open with a lot of fun stuff, order everything by 'Object Type' and find SqlConnection.

Double click your SqlConnection line.

Click the first line (assuming you have one) in the new portion of the window, then click 'Referenced Objects' at the bottom section of the window.

It should show you a bunch of objects, find SqlConnectionString, and drill down into it.

You should see several string objects. Go ahead and hover each one, one will be your password.

Of course, the string object on the SqlConnection itself will also be a connection string and have your password in it.

Bottom line: the client computer is always compromised in your eyes. You can do no more to secure it at this point, with an instance of Visual Studio (which this can be done more effectively with a separate programme that hides on the client computer) I can view your connection string and extract it with little to no effort required. (This whole process takes ~10 seconds once you have used the 'Break All' on the programme.)

My advice to you: don't take unnecessary security precautions to ensure your work is 'safe in memory' -- there's no such thing. All memory can be dumped, and even if it's only in memory for ~0.00001ms, an attacker can still find it. Just worry about building your application, let the memory security be handled by the user. :)

On to the code review:

• YesNo is a bad variable name; C# rules are camelCase for local members. It should also be named something related to what it does: windowsCredentials.

• You say data sanitization isn't important, but what happens if the user enters a semicolon then another ConnectionString property? You should always validate input.

• You should be using the SQLCon, not creating it like you are. (using (var connection = new SqlConnection(ConnectionString))), same with SqlCommand and SqlDataReader if/when/where you use them.

Why?

The SqlConnection type is an IDisposable - that means, it's capable of being automatically closed and deallocated.

In the case of SqlConnection, IIRC there are unmanaged objects that need disposed, cleared and cleaned up before the connection is done. There are resources it uses that are not automatically freed.

If you look at the API of SqlConnection, it has a Close() method which indicates that it will disconnect from the database and free it's resources. This is important because if you forget the Close() call, then your database connection will hang open. This has performance implications for your programme, database and network.

So, we use using (...) construct for it which is a fancy way of writing:

var connection = new SqlConnection();
try
{
    /* Inner code here */
}
finally
{
    ((IDisposable)connection).Dispose();
}

The Dispose() method of SqlConnection then handles closing and flushing all the connection information.

On to your requests:

• From a security perspective, would this code be acceptable in a SQL Importer?

As far as security goes - maybe. I'm not a security expert, but I would be wary of having no input validation here. At the very least, do not allow input with a semicolon (;), as that can totally mess with your connection string.

• Besides data sanitation, is there a best practice somewhere that I'm missing in this?

I mentioned them above, but generally speaking you're trying to solve a problem that we have accepted as a non-issue. Worry about the features of the application, then about the security. Your job as a programmer is not to secure the information in RAM, but to secure it:

• On disk / file system

• In the database

• Over the network

Don't worry so much about the password sitting in memory. Yes, it's generally safer to keep it in memory as shortly as possible, but that goes along with Single-Responsibility Principle, the Principle of Least Astonishment, etc. Your password (and all variables) should be as short-lived as possible. (I.e. Don't do like the old C code used to with 100 variables defined at the top of the file.)

I have turned this answer into a guide on my blog for further explanation.

StackExchange Code Review Q#151581, answer score: 6