bitwarden/server
1
0
Fork 0
mirror of https://github.com/bitwarden/server synced 2026-01-04 09:33:40 +00:00
Code Issues Releases Wiki Activity

Addressed Claudebot findings

Browse Source
This commit is contained in:
Mark Kincaid
2025-11-10 16:46:44 -08:00
parent 7318129168
commit 38741bfaea
9 changed files with 813 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
util/Seeder/Migration/CsvHandler.cs
View File

@@ -105,7 +105,15 @@ public class CsvHandler(CsvSettings settings, ILogger<CsvHandler> logger)
Delimiter = delimiterUsed,
HasHeaderRecord = _settings.IncludeHeaders,
Encoding = _encoding,
BadDataFound = null, // Ignore bad data
BadDataFound = context =>
{
// Log malformed CSV data to prevent silent data corruption
_logger.LogWarning(
"Bad CSV data at row {Row}, field {Field}: {RawRecord}",
context.Context.Parser.Row,
context.Field,
context.RawRecord);
},
TrimOptions = CsvHelper.Configuration.TrimOptions.None // Don't trim anything
};

22
util/Seeder/Migration/Databases/MariaDbImporter.cs
View File

@@ -26,15 +26,21 @@ public class MariaDbImporter(DatabaseConfig config, ILogger<MariaDbImporter> log
{
try
{
// Build connection string with redacted password for safe logging
var safeConnectionString = $"Server={_host};Port={_port};Database={_database};" +
$"Uid={_username};Pwd={DbSeederConstants.REDACTED_PASSWORD};" +
$"ConnectionTimeout={DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT};" +
$"CharSet=utf8mb4;AllowLoadLocalInfile=false;MaxPoolSize={DbSeederConstants.DEFAULT_MAX_POOL_SIZE};";
// SECURITY: Use MySqlConnectionStringBuilder to safely construct connection string
var builder = new MySqlConnectionStringBuilder
{
Server = _host,
Port = (uint)_port,
Database = _database,
UserID = _username,
Password = _password,
ConnectionTimeout = (uint)DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT,
CharacterSet = "utf8mb4",
AllowLoadLocalInfile = false,
MaximumPoolSize = (uint)DbSeederConstants.DEFAULT_MAX_POOL_SIZE
};
var actualConnectionString = safeConnectionString.Replace(DbSeederConstants.REDACTED_PASSWORD, _password);
_connection = new MySqlConnection(actualConnectionString);
_connection = new MySqlConnection(builder.ConnectionString);
_connection.Open();
_logger.LogInformation("Connected to MariaDB: {Host}:{Port}/{Database}", _host, _port, _database);

42
util/Seeder/Migration/Databases/PostgresImporter.cs
View File

@@ -27,13 +27,19 @@ public class PostgresImporter(DatabaseConfig config, ILogger<PostgresImporter> l
{
try
{
var safeConnectionString = $"Host={_host};Port={_port};Database={_database};" +
$"Username={_username};Password={DbSeederConstants.REDACTED_PASSWORD};" +
$"Timeout={DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT};CommandTimeout={DbSeederConstants.DEFAULT_COMMAND_TIMEOUT};";
// SECURITY: Use NpgsqlConnectionStringBuilder to safely construct connection string
var builder = new NpgsqlConnectionStringBuilder
{
Host = _host,
Port = _port,
Database = _database,
Username = _username,
Password = _password,
Timeout = DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT,
CommandTimeout = DbSeederConstants.DEFAULT_COMMAND_TIMEOUT
};
var actualConnectionString = safeConnectionString.Replace(DbSeederConstants.REDACTED_PASSWORD, _password);
_connection = new NpgsqlConnection(actualConnectionString);
_connection = new NpgsqlConnection(builder.ConnectionString);
_connection.Open();
_logger.LogInformation("Connected to PostgreSQL: {Host}:{Port}/{Database}", _host, _port, _database);
@@ -302,6 +308,7 @@ public class PostgresImporter(DatabaseConfig config, ILogger<PostgresImporter> l
var paramNum = idx + 1;
var colType = validColumnTypes[idx];
// Cast to appropriate type if needed - PostgreSQL requires explicit casts for text to other types
// SECURITY: Use explicit allowlist to prevent potential SQL injection through compromised schema
return colType switch
{
// UUID types
@@ -328,8 +335,27 @@ public class PostgresImporter(DatabaseConfig config, ILogger<PostgresImporter> l
// Boolean type
"boolean" => $"${paramNum}::boolean",
// Default - no cast needed for text types
_ => $"${paramNum}"
// Text and string types (no cast needed)
"text" => $"${paramNum}",
"character varying" => $"${paramNum}",
"varchar" => $"${paramNum}",
"character" => $"${paramNum}",
"char" => $"${paramNum}",
// Binary types
"bytea" => $"${paramNum}::bytea",
// JSON types
"json" => $"${paramNum}::json",
"jsonb" => $"${paramNum}::jsonb",
// Array types (common cases)
"text[]" => $"${paramNum}::text[]",
"integer[]" => $"${paramNum}::integer[]",
"uuid[]" => $"${paramNum}::uuid[]",
// Reject unknown types to prevent potential schema-based attacks
_ => throw new InvalidOperationException($"Unsupported PostgreSQL column type '{colType}' for column '{col}'. Type must be explicitly allowed in the PostgresImporter allowlist.")
};
});
var insertSql = $"INSERT INTO \"{actualTableName}\" ({string.Join(", ", quotedColumns)}) VALUES ({string.Join(", ", placeholders)})";

22
util/Seeder/Migration/Databases/SqlServerExporter.cs
View File

@@ -26,14 +26,18 @@ public class SqlServerExporter(DatabaseConfig config, ILogger<SqlServerExporter>
{
try
{
var safeConnectionString = $"Server={_host},{_port};Database={_database};" +
$"User Id={_username};Password={DbSeederConstants.REDACTED_PASSWORD};" +
$"TrustServerCertificate=True;" +
$"Connection Timeout={DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT};";
// SECURITY: Use SqlConnectionStringBuilder to safely construct connection string
var builder = new SqlConnectionStringBuilder
{
DataSource = $"{_host},{_port}",
InitialCatalog = _database,
UserID = _username,
Password = _password,
TrustServerCertificate = true,
ConnectTimeout = DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT
};
var actualConnectionString = safeConnectionString.Replace(DbSeederConstants.REDACTED_PASSWORD, _password);
_connection = new SqlConnection(actualConnectionString);
_connection = new SqlConnection(builder.ConnectionString);
_connection.Open();
_logger.LogInformation("Connected to SQL Server: {Host}/{Database}", _host, _database);
@@ -320,12 +324,14 @@ public class SqlServerExporter(DatabaseConfig config, ILogger<SqlServerExporter>
var whereClause = string.Join(" OR ", textColumns.Select(col => $"[{col}] IS NOT NULL"));
// SECURITY: Use parameterized query for TOP clause to maintain consistent security patterns
var query = $@"
SELECT TOP {sampleSize} {columnList}
SELECT TOP (@SampleSize) {columnList}
FROM [{tableName}]
WHERE {whereClause}";
using var command = new SqlCommand(query, _connection);
command.Parameters.AddWithValue("@SampleSize", sampleSize);
using var reader = command.ExecuteReader();
var sampleData = new List<object[]>();

18
util/Seeder/Migration/Databases/SqlServerImporter.cs
View File

@@ -28,14 +28,18 @@ public class SqlServerImporter(DatabaseConfig config, ILogger<SqlServerImporter>
{
try
{
var safeConnectionString = $"Server={_host},{_port};Database={_database};" +
$"User Id={_username};Password={DbSeederConstants.REDACTED_PASSWORD};" +
$"TrustServerCertificate=True;" +
$"Connection Timeout={DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT};";
// SECURITY: Use SqlConnectionStringBuilder to safely construct connection string
var builder = new SqlConnectionStringBuilder
{
DataSource = $"{_host},{_port}",
InitialCatalog = _database,
UserID = _username,
Password = _password,
TrustServerCertificate = true,
ConnectTimeout = DbSeederConstants.DEFAULT_CONNECTION_TIMEOUT
};
var actualConnectionString = safeConnectionString.Replace(DbSeederConstants.REDACTED_PASSWORD, _password);
_connection = new SqlConnection(actualConnectionString);
_connection = new SqlConnection(builder.ConnectionString);
_connection.Open();
_logger.LogInformation("Connected to SQL Server: {Host}/{Database}", _host, _database);

67
util/Seeder/Migration/Utils/IdentifierValidator.cs
View File

@@ -14,15 +14,34 @@ public static class IdentifierValidator
RegexOptions.Compiled
);
// Regex pattern for more restrictive validation (no leading underscores)
private static readonly Regex RestrictiveIdentifierPattern = new Regex(
@"^[a-zA-Z][a-zA-Z0-9_]*$",
RegexOptions.Compiled
);
// Maximum reasonable length for identifiers (most databases have limits around 128-255)
private const int MaxIdentifierLength = 128;
// SQL reserved keywords that should be rejected (common across SQL Server and PostgreSQL)
// SECURITY: Prevent identifiers that match SQL keywords to avoid injection attempts
private static readonly HashSet<string> SqlReservedKeywords = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
{
"SELECT", "INSERT", "UPDATE", "DELETE", "DROP", "CREATE", "ALTER", "EXEC", "EXECUTE",
"TABLE", "DATABASE", "SCHEMA", "INDEX", "VIEW", "PROCEDURE", "FUNCTION", "TRIGGER",
"WHERE", "FROM", "JOIN", "UNION", "ORDER", "GROUP", "HAVING", "INTO", "VALUES",
"SET", "AND", "OR", "NOT", "NULL", "IS", "AS", "ON", "IN", "EXISTS", "BETWEEN",
"LIKE", "CASE", "WHEN", "THEN", "ELSE", "END", "BEGIN", "COMMIT", "ROLLBACK",
"GRANT", "REVOKE", "DECLARE", "CAST", "CONVERT", "TRUNCATE", "MERGE"
};
/// <summary>
/// Validates a SQL identifier (table name, column name, schema name).
/// </summary>
/// <param name="identifier">The identifier to validate</param>
/// <param name="useRestrictiveMode">If true, disallow leading underscores and SQL reserved keywords</param>
/// <returns>True if the identifier is valid, false otherwise</returns>
public static bool IsValid(string? identifier)
public static bool IsValid(string? identifier, bool useRestrictiveMode = false)
{
if (string.IsNullOrWhiteSpace(identifier))
return false;
@@ -30,7 +49,13 @@ public static class IdentifierValidator
if (identifier.Length > MaxIdentifierLength)
return false;
return ValidIdentifierPattern.IsMatch(identifier);
// SECURITY: In restrictive mode, reject SQL reserved keywords
if (useRestrictiveMode && SqlReservedKeywords.Contains(identifier))
return false;
// Use appropriate pattern based on mode
var pattern = useRestrictiveMode ? RestrictiveIdentifierPattern : ValidIdentifierPattern;
return pattern.IsMatch(identifier);
}
/// <summary>
@@ -38,15 +63,35 @@ public static class IdentifierValidator
/// </summary>
/// <param name="identifier">The identifier to validate</param>
/// <param name="identifierType">The type of identifier (e.g., "table name", "column name")</param>
/// <param name="useRestrictiveMode">If true, disallow leading underscores and SQL reserved keywords</param>
/// <exception cref="ArgumentException">Thrown when the identifier is invalid</exception>
public static void ValidateOrThrow(string? identifier, string identifierType = "identifier")
public static void ValidateOrThrow(string? identifier, string identifierType = "identifier", bool useRestrictiveMode = false)
{
if (!IsValid(identifier))
if (string.IsNullOrWhiteSpace(identifier))
{
throw new ArgumentException(
$"Invalid {identifierType}: identifier is null or empty.",
nameof(identifier)
);
}
if (useRestrictiveMode && SqlReservedKeywords.Contains(identifier))
{
throw new ArgumentException(
$"Invalid {identifierType}: '{identifier}' is a SQL reserved keyword.",
nameof(identifier)
);
}
if (!IsValid(identifier, useRestrictiveMode))
{
var requirements = useRestrictiveMode
? "must start with a letter (no underscores), contain only letters, numbers, and underscores"
: "must start with a letter or underscore, contain only letters, numbers, and underscores";
throw new ArgumentException(
$"Invalid {identifierType}: '{identifier}'. " +
$"Identifiers must start with a letter or underscore, " +
$"contain only letters, numbers, and underscores, " +
$"Identifiers {requirements}, " +
$"and be no longer than {MaxIdentifierLength} characters.",
nameof(identifier)
);
@@ -58,12 +103,13 @@ public static class IdentifierValidator
/// </summary>
/// <param name="identifiers">The identifiers to validate</param>
/// <param name="identifierType">The type of identifiers (e.g., "column names")</param>
/// <param name="useRestrictiveMode">If true, disallow leading underscores and SQL reserved keywords</param>
/// <exception cref="ArgumentException">Thrown when any identifier is invalid</exception>
public static void ValidateAllOrThrow(IEnumerable<string> identifiers, string identifierType = "identifiers")
public static void ValidateAllOrThrow(IEnumerable<string> identifiers, string identifierType = "identifiers", bool useRestrictiveMode = false)
{
foreach (var identifier in identifiers)
{
ValidateOrThrow(identifier, identifierType);
ValidateOrThrow(identifier, identifierType, useRestrictiveMode);
}
}
@@ -71,9 +117,10 @@ public static class IdentifierValidator
/// Filters a list of identifiers to only include valid ones.
/// </summary>
/// <param name="identifiers">The identifiers to filter</param>
/// <param name="useRestrictiveMode">If true, disallow leading underscores and SQL reserved keywords</param>
/// <returns>A list of valid identifiers</returns>
public static List<string> FilterValid(IEnumerable<string> identifiers)
public static List<string> FilterValid(IEnumerable<string> identifiers, bool useRestrictiveMode = false)
{
return identifiers.Where(IsValid).ToList();
return identifiers.Where(id => IsValid(id, useRestrictiveMode)).ToList();
}
}