Logging Hours
If you are starting a new company, or just want to track hours, there is an excelent tool called clockify.
You can create a project, or a client and then log each step. Once you have a breakdown your hours, you can begin to build more accurate estimates and know exactly what you need to charge.
Git Hub Clone and Branching
Let’s open a git hub account and fork DVTA. We can work from here and make the secure version.
First download GitHub desktop. Under the code drop down you will see the option to download. Follow the directions and then clone the master to your preferred location.
We need to branch. At the bottom of visual studio, click master. Add a new branch like FixingMisingFiles. After we make a change, we will check the branch in and merge it into master. We will repeat this option for each group of like changes. It is important to grasp this concept, as we will be developing a DevOps process soon that will build upon this. I will use <<Branch>> to indicate this process of branch and merge.
Fixing the assembly
The main set of code is missing key files. We will need to use dnSPY or JustDecompile to extract those files and place them in the solution. Close Visual Studio. Go to the main projects folders and create a preferences folder. We places resources here. Program.cs goes in the main folder. Delete textbox.cs, I can see no where it is used. In DBAccess, Add the properties folder. Open VS and add a new assembly file.
Let’s upgrade the .NET version. Verify it builds successfully. We can comment out the exit if debugging code and verify the program runs. Enable the textbox and button so we can configure the server. My solution would not build as Register.Designer.cs. However DBAccess built. To debug, let’s delete the Register files and all references to them. There should be only once reference in Login.cs. The solution now builds.
<<Branch>>
Branch IDEChangesAndConsoleOutput. For now, let’s go commenting out the Writeline() in case we add secure logging. I like to add a comment like /* TODO: Add logging */. Then I can search on TODO and go through making changes.
Open Solution Explorer and right click on each project. Make the changes for each project in the gallery below.
Make all the recommended changes, like standard C# naming rules. After making the above changes I had 30 warnings and 34 messages. We can also use XML commenting to provide intelisense and documentation. An example follows:
/// Maintenance:
/// Code Reviewer: JSecOps 10/2023
/// Added XML commenting for demo.
/// Refactored removing unnecessary using and code.
/// Fixed inject vulnerability in csv file.
/// <href>https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/recommended-tags</href>
/// </summary>
using System.Text;
using System.Data;
namespace DVTA
{
/// <summary>
/// Static Class containing method WriteToCsvFile.
/// </summary>
public static class DataTableToCsv
{
/// <summary>
/// Export a datatable to a file.
/// </summary>
/// <param name="dataTable">The datatable with exporting data.</param>
/// <param name="filePath">The folder path to place the csv file.</param>
public static void WriteToCsvFile(this DataTable dataTable, string filePath)
{
// Method code removed for brevity
}
}
}<<Branch>>
Next we create a user table where we will hash the password. We can move users over as they login.
-- Our original users table
CREATE TABLE [dbo].[users2](
[id] [int] IDENTITY(0,1) NOT NULL,
[username] [varchar](100) NOT NULL,
[password] [varchar](100) NOT NULL,
[email] [varchar](100) NULL,
[isadmin] [int] NULL,
PRIMARY KEY CLUSTERED
(
[id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY]Now some modifications. Here we are going for least modifications. In reality this should be done completely differently.
-- First let's create a role table
create table dbo.roles (
[id] [int] IDENTITY(0,1) NOT NULL,
[Roles] nvarchar(100) NOT NULL,
Primary key ([id] asc))
insert into dbo.roles (Roles) values ('No Login') -- started at 0 for no role
insert into dbo.roles (Roles) values ('Admin') -- keeping admin 1
insert into dbo.roles (Roles) values ('Standard User') -- a standard userTo keep the sample minimal, going with one user table, just changed to user2 and a history table.
-- we create a table that has actions performed by a user
create table dbo.useraction (
[id] [int] IDENTITY(0,1) NOT NULL,
[ActionDescription] nvarchar(100) NOT NULL,
Primary key ([id] asc))
go
insert into dbo.useraction ([ActionDescription]) values ('Initial Account Creation')
-- Our new user tables. Notice we moved RoleId to be a foreign key.
-- Say we had 10,000 users and didn't want to change the password hash all at once, we could
-- migrate on login with this approach. Just giving options.
CREATE TABLE [dbo].[users2](
[id] [int] IDENTITY(1,1) NOT NULL,
[username] [varchar](100) NOT NULL,
[password] [varchar](100) NOT NULL,
[email] [varchar](100) NOT NULL,
[RoleId] [int] NOT NULL default 0,
Foreign Key([RoleId]) references Roles(Id),
PRIMARY KEY CLUSTERED
(
[id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY]
--Now if we make a change to user2, we have no record
go
CREATE TABLE [dbo].[users2history](
[id] [int] IDENTITY(1,1) NOT NULL,
[UserId] [int], --* start copy of table
[username] [varchar](100) NOT NULL,
[password] [varchar](100) NOT NULL,
[email] [varchar](100) NOT NULL,
[RoleId] [int] not null,
[ActionId] [int] not null, --* end copy of table
Foreign Key([RoleId]) references Roles(Id),
Foreign Key([ActionId]) references useraction(Id),
Foreign Key([UserId]) references users2(Id),
[ActionDate] DATETIME2 GENERATED ALWAYS AS ROW START NOT NULL, -- Voodoo begins here
[ActionDate2] DATETIME2 GENERATED ALWAYS AS ROW END HIDDEN NOT NULL, -- required but hidden
PERIOD FOR SYSTEM_TIME ([ActionDate],[ActionDate2]), -- period used for our date
PRIMARY KEY CLUSTERED
([id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY]
Now here as the user changes we log the change. We use period and a hidden column to autogenerate our date.
Upgrade to the newest framework.
Change to debug and attempt to build. I have a failure missing Register.Designer.cs. However DBAccess built. To debug, let’s delete the Register files and all references to them. There should be only once reference in Login.cs. The solution now builds.
Next we set both projects properties to the following settings. Comment out the time error and for now remove the exception. We will be making an alternative. Now go through all the messages and fix the naming violations and other violations. Note that several forms have empty events. Open the form and in properties remove the events. Delete all code necessary. Some will have events that don’t show. Delete the empty methods and then find the code in the resources where the event is wired up and delete it. Clean up the code and remove spaces while we are in here.
Note: Within DBAccess we will be doing a major overhall, leave the DecryptPassword() method alone. You should have a working program now (minus our register).
The next stage I like, and this isn’t totally necessary, to go through each file, remove unnecessary usings, remove white space, and then comment what I think is going on. We can also run Analyze -> Code Clean Up to do some of the same.
Verify the code builds and has been cleaned up. At this point, you can run Code Metrics if you like to get an idea of the complexity of the program. We will be going more indepth with this in the future.
Make choice? EF or sql params.
With a cleaned up source code, let’s install a few visual studio extensions just to run and see what they find. In Extensions -> Search simply type “security” and you will find a few. Let’s install Puma Scan Community Ed. Close and reopen VS2022. Now we have security errors.
Let’s recode the entire DBAccess project in a way to stop these sql injection attacks and make the code more clean. First I create a new folder called refactor and copy DBAccess.cs. I pull out the crypto method into it’s own code.
First implement iDisposible on the interface and that way it can be called wrapped in using statements.
-- A slightly less terrible way using parameters
/// <summary>
/// Method to check if a login is valid.
/// TODO: encrypt password
/// </summary>
/// <param name="clientusername">Username</param>
/// <param name="clientpassword">Password</param>
/// <returns>SqlDataReader object</returns>
public SqlDataReader CheckLogin(string clientusername, string clientpassword)
{
//We should really avoid the use of * and return the exact columns back we need.
string sqlcmd = "SELECT username, password FROM users where username=@username and
password=@password";
SqlCommand cmd = new SqlCommand(sqlcmd, conn);
cmd.Parameters.AddWithValue("@username", clientusername);
cmd.Parameters.AddWithValue("@password", clientpassword);
SqlDataReader dtr = cmd.ExecuteReader();
return dtr;
}
-- another way to use parameters
/// <summary>
/// Register a new user.
/// TODO: encrypt password
/// </summary>
public bool RegisterUser(string clientusername, string clientpassword, string
clientemailid)
{
bool output = false;
//again list the columns so if the table changes we can still insert
string sqlquery = "insert into users values(@clientusername, @clientpassword,
@clientemailid, 0)";
SqlCommand cmd = new SqlCommand(sqlquery, conn);
cmd.Parameters.Add(new SqlParameter("clientusername", clientusername));
cmd.Parameters.Add(new SqlParameter("clientpassword", clientpassword)); //encrypt
cmd.Parameters.Add(new SqlParameter("clientemailid", clientemailid));
try
{
output = cmd.ExecuteNonQuery() > 0 ? true : false;
}
catch (Exception e)
{
//Log this don't print to the console Console.WriteLine(e);
}
return output;
}Now for the encryption, let’s implement something better.
public static string Decrypt(byte[] decryptThis)
{
using (Aes myAes = Aes.Create()) //this creates a new key and IV
// also store in an encrypted safe area
{
// Decrypt the bytes to a string.
string decryptedstring = DecryptStringFromBytes_Aes(decryptThis, myAes.Key,
myAes.IV);
return decryptedstring;
}
}
static string DecryptStringFromBytes_Aes(byte[] cipherText, byte[] Key, byte[] IV)
{
// Check arguments.
string plaintext = null;
// Create an Aes object
// with the specified key and IV.
using (Aes aesAlg = Aes.Create())
{
aesAlg.Key = Key;
aesAlg.IV = IV;
// Create a decryptor to perform the stream transform.
ICryptoTransform decryptor = aesAlg.CreateDecryptor(aesAlg.Key, aesAlg.IV);
// Create the streams used for decryption.
using (MemoryStream msDecrypt = new MemoryStream(cipherText))
{
using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, decryptor,
CryptoStreamMode.Read))
{
using (StreamReader srDecrypt = new StreamReader(csDecrypt))
{
plaintext = srDecrypt.ReadToEnd();
}
}
}
}
return plaintext;
}Secrets Storage
The Key and Initialization Vector (IV) are stored in the config file. There is a way to encrypt config data, but it can easily be decrypted on the machine it was encrypted on, since that is the program we have access to, this is unacceptable. This is still a better option than hard coding the credentials in the code.
A good option here is the Windows Credential Manager. In the CM, we can store passwords, keys, salts, basically anything we need to be protected. Using a nuGet package, we can easily retrieve data as such
using CredentialManagement;
var cm = new Credential { Target = "TheApp", Username = "TheUsername" };
if (cm.Load())
{
var password = cm.Password;
}Two other options are Git Secrets or Azure Key Vault. Both help prevent the leaking of secrets from source code. Finally, environent variables are an option, though I am not found of this approach.
Environment.SetEnvironmentVariable("MY_SECRET", "mysecretpassword"); // Set
string mySecret = Environment.GetEnvironmentVariable("MY_SECRET"); // RetrievePreventing DLL Hijacking
When a program goes to load a DLL it will search in the following way
- The directory from which the application loaded
- The system directory
- The 16-bit system directory
- The Windows directory
- The current working directory (CWD)
- The directories that are listed in the PATH environment variable
To avoid this, call SetDllDirectory API by using an empty string (“”). If an application depends on loading a DLL from the current directory, please obtain the current working directory and use that to pass in a fully qualified path of LoadLibrary.
- Validate their applications for instances of nonsecure library loads. These include the following:
- The use of SearchPath to identify the location of a library or component.
- The use of LoadLibrary to identify the version of the operating system.
- Use fully qualified paths for all calls to LoadLibrary, CreateProcess, and ShellExecute where you can.
- Implement calls to SetDllDirectory with an empty string (“”) to remove the current working directory from the default DLL search order where it is required.
- Use the SetSearchPathModefunction to enable safe process search mode for the process. This moves the current working directory to the last place in the SearchPath search list for the lifetime of the process.
- Avoid using SearchPath to check for the existence of a DLL without specifying a fully qualified path, even if safe search mode is enabled, because this can still lead to DLL Preloading attacks.
Obfuscation
Originally we were able to set the server IP by altering the IL code. To make this much harder, for one a program would never be setup this way and two if the code is obfuscated, then it will be that much harder to find and enable the button.