Voici la ligne de code où je tronque les enregistrements de table. La valeur de la table provient du frontal. Dans mon scan Veracode, il montre une injection SQL. Comment puis-je éviter ça? Je ne peux pas créer de procédure stockée car la chaîne de connexion est dynamique là où je dois tronquer cette table. Existe-t-il une autre approche?
SqlCommand cmd = connection.CreateCommand(); cmd.Transaction = transaction; cmd.CommandText = "TRUNCATE TABLE " + tablename; cmd.ExecuteNonQuery();
3 Réponses :
Vous avez besoin de SQL dynamique:
string sql = @"
DECLARE @SQL nvarchar(150);
SELECT @SQL = 'truncate table ' + quotename(table_name) + ';'
FROM information_schema.tables
WHERE table_name = @table;
EXEC(@SQL);";
using (var connection = new SqlConnection("connection string here"))
using (var cmd = new SqlCommand(sql, connection))
{
cmd.Transaction = transaction;
cmd.Parameters.Add("@table", SqlDbType.NVarChar, 128).Value = tablename;
connection.Open();
cmd.ExecuteNonQuery();
}
C'est l'une des rares fois où le SQL dynamique rend les choses plus sécurisées, plutôt que moins. Mieux encore, si vous gérez également une table spéciale dans cette base de données répertoriant les autres tables, les utilisateurs sont autorisés à tronquer et à l'utiliser plutôt que information_schema pour valider le nom. L'idée de laisser les utilisateurs tronquer n'importe quoi est assez effrayante.
il ne créera pas de défaut d'injection SQL ?? car le nom de la table vient du front-end
@SarveshGupta Non, ce ne sera pas le cas puisque ce nom de table est paramétré ici
@SarveshGupta Ceci fait défiler l'entrée dans la base de données de manière sûre pour être sûr que vous avez un nom de table valide. Reste la question de savoir pourquoi vous autoriseriez n'importe quel nom de table à être sélectionné et complètement tronqué.
@SarveshGupta Ajout de plus de code pour le contexte. Vous pouvez maintenant voir où la table est passée en tant que valeur de paramètre sûre.
Le paramétrage @Rahul n'est pas ce qui le rend magiquement sûr ici. Ce sont les autres étapes
Aucune quantité de paramétrage n'aura d'importance avec le SQL dynamique s'il n'est pas correctement échappé. C'est ce que fait QUOTENAME . Tout dépend de cela.
@madreflection Aussi que j'ai extrait le nom de la table de la base de données. Si l'utilisateur entre quelque chose qui n'est pas une vraie table, il n'y a rien à citer. Les deux mis ensemble signifient que vous obtenez au moins une seule instruction SQL valide qui fait exactement ce que vous attendez. Si c'était moi, cependant, j'aurais également une table spéciale dans cette base de données qui répertorie un ensemble spécifique d'autres tables que les utilisateurs sont autorisés à tronquer.
voici un moyen de le masquer des outils d'analyse
private const string _sql = "VFJVTkNBVEUgVEFCTEU=";
. . . .
var temp = new { t = tablename };
cmd.CommandText =
Encoding.ASCII.GetString(Convert.FromBase64String(_sql)) + temp.t.PadLeft(temp.t.Length + 1);
security by obscurity
N'est-ce pas utiliser l'obscurité pour vaincre un problème de sécurité totalement légitime découvert par l'analyse de code?
@MartinSmith vous le paramétrez ou non, si vous obtenez quelque chose hors de la zone de texte et que vous concaténez en "Tronquer la table", ce n'est pas sécurisé. Ce qui est plus sécurisé, c'est si le contrôle est un combo, ou une zone de liste, s'il est vérifié par rapport à une liste de table, etc. Il s'agit ici strictement de vaincre un outil d'analyse
Pourquoi tenteriez-vous de «vaincre» un outil d'analyse que vous exécutez vous-même? N'exécutez pas d'analyse de code statique si vous pensez que cela n'en vaut pas la peine, ou si vous l'avez évaluée en contexte et que vous l'avez trouvée sûre, utilisez la méthode utilisée pour marquer comme sûre. Rendre le code très obscurci pour que personne ne puisse voir ce qu'il fait n'est pas une bonne solution
@MartinSmith Parlons de la situation actuelle. Le seul moyen plus ou moins sécurisé de le faire est lorsque vous obtenez une liste avec un identifiant dans le front-end, passez-la au back-end et comparez, disons, une table. Pas une table de schéma d'information mais une table personnalisée qui contient cet id et cette instruction tuncate réelle. Et de préférence, cet identifiant sera un Guid. Maintenant, les outils d'analyse se plaindront de toutes ces choses dont vous avez besoin, point final. Ces outils se plaignent que votre variable s'appelle password ou pwd . Alors oui, nous les appelons p_zz4rd ou quelque chose comme ça, juste pour que l'outil arrête de pleurnicher
Paramétré ou non, vous pouvez le rendre un peu plus sécurisé dans ce cas. Jamais totalement sécurisé. Pour cela, vous avez besoin
créer la table
TruncMappingdans la base de données où vous stockez
id guid
instruction varchar (300)
vos données ressembleront à
SOME-GUID-XXX-YYY, 'TRUNCATE TABLE TBL1'
listbox ou combobox avec du texte / une valeur comme "Customer Data" / "SOME-GUID-XXX-YYY" ExecuteScalar pour exécuter l'instruction Select de TruncMapping où id = @ 1 , où id sera paramétré GUID à partir de la valeur combo ExecuteNonQuery comme vous le faites maintenant, mais avec une chaîne récupérée de l'appel précédent. Votre outil d'analyse va probablement s'étrangler. S'il pense toujours que le code n'est pas sûr, vous pouvez le signaler en toute sécurité comme un faux positif car ce que vous exécutez provient de votre base de données sécurisée. L'attaquant potentiel n'a aucun moyen de saboter vos "tables non tuncatable" car elles ne sont pas répertoriées dans les tables TruncMapping .
Vous venez de créer une défense multicouche contre l'injection SQL.
vous pouvez marquer comme faux positif. Mais je suis d'accord, c'est une mauvaise décision. Pourquoi ne pouvez-vous pas créer de procédure (s)?
combien de tables pourraient éventuellement être envoyées à ce code? Je dirais de mettre la liste des tables possibles dans un tableau ou un jeu de hachage et que votre code s'assure que la valeur passée de "tablename" est là. Utilisez ensuite la valeur du tableau au lieu de la valeur transmise.
Ce n'est pas exactement un faux positif. Encore. Vous devez d'abord vous assurer que vous avez correctement échappé le nom.
ou utilisez un type énuméré ou une chaîne const pour "nom_table" au lieu d'une chaîne régulière
string sqlStatement = string.Format ("TRUNCATE TABLE [{0}]", tableName);
@TS comme je l'ai dit, la chaîne de connexion est dynamique là où je dois tronquer la table, nous encadrons la valeur de la chaîne de connexion à partir de la valeur de base de données qui provient du front-end et exécutons cette instruction tronquée dans ce contexte de chaîne de connexion, comment sp aidera et où nous mettons ce sp
juste une note latérale ... Je codais depuis longtemps en utilisant SQL et je n'ai jamais eu besoin de code d'application (surtout multi-niveaux!) pour tronquer une table, vous voudrez peut-être reconsidérer votre conception. À moins que vous n'écriviez un front-end pour un outil d'administration de base de données, je pense qu'il existe un meilleur moyen
@JoelFan Je comprends joe, ce n'est pas conçu par moi et son très vieux code, je viens de recevoir la tâche de résoudre ces problèmes, alors je voulais juste une aide efficace si nous pouvons faire quoi que ce soit sans repenser l'architecture entière
@SarveshGupta Chaque DB devra contenir ce SP, où vous passerez la crinière de table et exécuterez du code dynamique. Peu importe d'où vous vous connectez.