I have done the following...
private static IDbConnectionProvider CreateSqlConnectionProvider(DbConfig dbConfig)
{
return new QcDbConnectionProvider(() =>
{
SqlConnectionStringBuilder csBuilder = new SqlConnectionStringBuilder();
if (!string.IsNullOrEmpty(dbConfig.DataSource))
csBuilder.DataSource = dbConfig.DataSource;
if (!string.IsNullOrEmpty(dbConfig.Database))
csBuilder.InitialCatalog = dbConfig.Database;
.
.
.
.
return new SqlConnection(csBuilder.ConnectionString);
});
}
The client is using VERACODE tool for doing code analysis and the VERACODE has detected a flaw "Untrusted initialization" at
return new SqlConnection(csBuilder.ConnectionString);
Also, the dbConfig is being initialized as shown below...
DbConfig configDbConfig = new DbConfig
{
Database = codeFile.ConfigurationDb,
DataSource = codeFile.DataSource,
IntegratedSecurity = sqlCredentials.UseWindowsAuthentication ? 1 : 0,
UserId = sqlCredentials.UseWindowsAuthentication ? null : sqlCredentials.SqlUserName,
ClearTextPassword = sqlCredentials.UseWindowsAuthentication ? null : sqlCredentials.SqlUserPassword
};
What else I need to do in order to fix this flaw? Also as per this link, I am creating the connection string using the SqlConnectionStringBuilder which is safe of creating the connection string.
Thanks in advance...
Description for Untrusted initialization issue is:
Applications should be reluctant to trust variables that have been initialized outside of its trust boundary. Untrusted initialization refers to instances in which an application allows external control of system settings or variables, which can disrupt service or cause an application to behave in unexpected ways. For example, if an application uses values from the environment, assuming the data cannot be tampered with, it may use that data in a dangerous way.
In your case you're reading data for dbConfig from file:
if (TryReadCodeFile(configurationProfileFile...)) {
DbConfig configDbConfig = new DbConfig...
}
Note that warning you get should also come with a line number (to circumscribe erroneous code). Almost everything in code you posted can generate this issue (I don't see where sqlCredentials comes from but it may even be another source of security problems if they're in clear text - or code to decrypt is accessible in your application).
From cited paragraph: "...application allows external control of system settings or variables, which can disrupt service...". This is the core of this issue: if your application uses external data without a direct control over them then its behavior can be changed modifying that data. What these external data are? List is all but not exhaustive:
- Environment variables (for example to resolve a path to another file or program) because user may change them. Original files aren't touched but you read something else.
- Paths (to load code or data) because user may redirect to something else (again original files aren't touched but you read something else).
- Support files because user can change them (in your case, for example, to point to another server and/or catalog).
- Configuration files because user can change them (same as above).
- Databases because they may be accessible to other users too and they may be changed (but they may be protected).
How a malicious user may use this? Imagine each user is connected to a different catalog (according to their rule in organization). This cannot be changed and it's configured during installation. If they can have access to your configuration files they may change catalog to something else. They may also change DB host name to a tunnel where they may sniff data (if they have physical access to someone else's machine).
Also note that they also say "...assuming the data cannot be tampered with, it may use that data in a dangerous way". It means if, for example, your application runs on a web server and physical access is secured then you may consider that data safe.
Be aware your application will be secure as less secure item in your whole system. Note that to make an application safe (I know, this term is pretty vague) to encrypt password is not enough.
If support files may be manipulated then best thing you can do is to encrypt them with a public/private key encryption. A less optimal solution is to calculate a CRC or hash (for example) you'll apply to configuration files before you use them (they are able to change them but your application will detect this issue).
To summarize: you can ignore this issue but you have to prove your customer that data you rely on cannot be tampered. You can reasonably prove if at least one of these conditions is satisfied:
1) System where support files reside is not accessible by anyone else than your application. Your application security cannot be higher than system security.
2) Your support files are valid per-machine (to avoid copies between different machines) and they're encrypted in a way they cannot be changed (intentionally or not) by anyone.
3) Your support files are valid per-machine and they're hashed in a way your application can detect external changes.
4) It doesn't matter what users do with your configuration files, application itself cannot change its behavior because of that (for example it's a single installation where only one DB and one catalog exist).
The most important for connection strings is how they are stored. If they are stored in plaintext, this poses a security risk. So, it is advisable to store them in encrypted format and in application decrypt and use it.
