Discussion :

[JeanYvette]

Bonjour tout le monde,

Désolé si le sujet a déjà été traité mais j'ai l'impression de pas trouver de réponses exact ou autre (je me sens nul ohlala je pleure)
Je vous présente le contexte, j'ai fait une bdd sous sql server et pour la tester je le fais en asp (site rapide, pas de design, juste du fonctionnel). Je ne fonctionne que par fonction table ou scalar, et par procédure stockée. J'appelle tout cela dans mes différentes pages asp, tout va bien dans le meilleur des mondes
Mais un jour j'ai eu une vision (bon ok jsuis juste allé sur le site de la CNIL) et là, horreur LES INJECTIONS SQL !!! *tremble*
Ma question est simple, est ce que ce système de procédure stockées ou function pour toutes opérations suffisent à palier aux injections sql ???

Je prie le grand Bouddha pour que oui mais bon, je suis pas trop sûr !

[JeanYvette]

Juste un petit up avant de me faire insulter

Dans les appels (string requete = "") j'ai fait en sorte que tout ce qui peut être APRES un ; soit supprimé

[SQLpro]

est ce que ce système de procédure stockées ou function pour toutes opérations suffisent à palier aux injections sql ???

Absolument pas !

Pour se protéger des injections de SQL (c'est plus facile si vous n'avez fait que du code côté serveur), il faut :
1) mettre en place la sécurité base sur des privilèges en rapport à des utilisateurs SQL / compte de connexion. Cela sera grandement facilité si vous avez découpé votre base en de multiples schémas SQL et je vous conseille de jouer sur les rôles
2) dans les routines (procédures et déclencheurs), qui propose du SQL dynamique (facilement identifiable par "EXEC (" ou sp_executesql) il faut vérifier vos variables et voir si elles ne contiennent pas du code maligne.... La plupart du temps c'est trivial, par une comparaisons aux méta données. Dans les autres cas, faire une fonction générique qui repère certains motifs, comme :
EXECUTE
EXEC
sp_executesql.
[
]
%
_
"
;
--
/*
*/
xp_
sp_

Vous pouvez même faire une table de tous ces motifs plutôt que de les mettre en dur.

Enfin, ne renvoyez aucune erreur en cas de détection d'une possible injection et tracez ce qui a été envoyé. Cela vous permettra de bloquer l'adresse IP de l'expéditeur.
Dans le même esprits, ne renvoyez jamais les erreurs telles qu'elle. Enregistrez les erreurs pour les analyser plus tard et renvoyez une erreur générique...

A +

[JeanYvette]

Pour les privilège c'est bon

Pour les variable je suis pas sur de bien comprendre, si je fais par exemple :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
exec InsertionUsers @NomUtilisateur = '" + TextBoxNom.Text + "', @PrenomUtilisateur ='" + TextBoxPrenom.Text + "', @NumeroRueUtilisateur = " 
+ TextBoxNumRue.Text + ", @NomRueUtilisateur = '" + TextBoxNomRue.Text + " ' "

On peut mettre, si je comprends ce que vous dites, des requêtes dans les textbox.text qui seraient au final exécutés par le sgbd ??


Pour ce qui est du traçage j'ai une table qui historise les requêtes fait par les utilisateurs, la date, et l'ip de connection (pour pouvoir faire une rollback plus tard) est-ce une bonne méthode ?

[SQLpro]

Pour les privilège c'est bon

Pour les variable je suis pas sur de bien comprendre, si je fais par exemple :

exec InsertionUsers @NomUtilisateur = '" + TextBoxNom.Text + "', @PrenomUtilisateur ='" + TextBoxPrenom.Text + "', @NumeroRueUtilisateur = " + TextBoxNumRue.Text + ", @NomRueUtilisateur = '" + TextBoxNomRue.Text + " ' "

Ne sachant pas avec quel outil vous développez vos IHM, je suis en peine de vous aider. Le mieux serait que cet outil soit capable de faire des requêtes paramètrées.
De mon temps (mon dieu, j'ai l'air d'un vieux con !!!) on faisait :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
theQuery.text := "exec InsertionUsers @NomUtilisateur = :param1 , @PrenomUtilisateur = :param2 , @NumeroRueUtilisateur = :param3 , @NomRueUtilisateur = :param4;";
theQuery.params[1] := TextBoxNom.Text;
theQuery.params[2] := TextBoxPrenom.Text;
theQuery.params[3] := TextBoxNumRue.Text;
theQuery.params[4] := TextBoxNomRue.Text;
theQuery.execute();

Ce qui permettait déjà de limiter la casse en ayant prévu des paramètres typés !

On peut mettre, si je comprends ce que vous dites, des requêtes dans les textbox.text qui seraient au final exécutés par le sgbd ??


Pour ce qui est du traçage j'ai une table qui historise les requêtes fait par les utilisateurs, la date, et l'ip de connection (pour pouvoir faire une rollback plus tard) est-ce une bonne méthode ?

Dans la procédure, pour chacun des paramètres passés, vérifier s'il ne contient pas des caractères ou motifs illicites...

Pour cela faite par exemple le module suivant :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
USE MaBase
GO
 
CREATE SCHEMA S_SECURITY
CREATE TABLE T_EXCLUDE_INJECTION_PATTERN_EIP
(EIP_ID         SMALLINT IDENTITY PRIMARY KEY,
 EIP_PATTERN    VARCHAR(16) NOT NULL UNIQUE);
GO

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
INSERT INTO S_SECURITY.T_EXCLUDE_INJECTION_PATTERN_EIP VALUES
('EXECUTE'), 
('EXEC'), 
('sp_executesql'), 
('['), 
(']'), 
('%'), 
('_'), 
('"'), 
(''''), 
(';'), 
('--'), 
('/*'), 
('*/'), 
('xp_'), 
('sp_');
GO

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
CREATE FUNCTION S_SECURITY.IS_INJECTION_PATTERN (@WORD NVARCHAR(max))
RETURNS BIT
BEGIN
   IF EXISTS(SELECT *
                FROM   S_SECURITY.T_EXCLUDE_INJECTION_PATTERN_EIP 
		        WHERE  @WORD LIKE N'%' + EIP_PATTERN + '%')
      RETURN 1;
   RETURN 0;
END;
GO

Dans vos procédures, au tout début, juste après le SET NOCOUNT ON; ajoutez un test de vos variables :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
IF S_SECURITY.IS_INJECTION_PATTERN(@NomUtilisateur) = 1
   RETURN;
IF S_SECURITY.IS_INJECTION_PATTERN(@PrenomUtilisateur) = 1
   RETURN;
...

ne testez que les variables littérales !

A +

[JeanYvette]

D'accord je viens de comprendre, un énorme merci à vous pour le coup
Je vais faire ce que vous avez dit de ce pas

ah mais oui du coup j'avais fait un appel de requête de ce style là :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
 string requete = "exec InsertionPourPatient @NomUtilisateur = '" + TextBoxNom.Text + "', @PrenomUtilisateur ='" + TextBoxPrenom.Text + "', @NumeroRueUtilisateur = " + TextBoxNumRue.Text + ", @NomRueUtilisateur = '" + TextBoxNomRue.Text + "', @VilleUtilisateur = '" + TextBoxVille.Text + "', @Identifiant='" + TextBoxSecuSociale.Text + "', @Metier  = '" +TextBoxMetier.Text+ " ' "

Mais ca marche plus avec cette histoire de double quotes qui sont interdites via la table des caractères c'est ça ?

[SQLpro]

C'est plutôt que votre outil de développement doit proposer les bons objets....
Quel est cet outil ? .net ? java ? PHP ? ...

Et pour les tests, uniquement ceux qui font du SQL dynamique. Dans le cas d'une requête paramétrée, il n'y a aucun intérêt de faire des tests.

A +

[JeanYvette]

Je développe en asp.net sur visual studio 2013

D'accord donc un appel de procédure comme celui que j'ai copié là n'a pas besoin d'être testé parce que la procédure possède des paramètres ? ?

[SQLpro]

Je développe en asp.net sur visual studio 2013

D'accord donc un appel de procédure comme celui que j'ai copié là n'a pas besoin d'être testé parce que la procédure possède des paramètres ? ?

Non, il faut faire une requête paramétrée ! Ce que vous faites actuellement c'est du raboutage de chaines de caractères donc injection assurée au niveau ASP !

Lisez ceci :
http://forums.asp.net/t/1568268.aspx...erized+Queries

A +

[JeanYvette]

Désolé de la réponse un peu tardive
Merci beaucoup sur ce lien je me suis pensé dessus et je le ferais faire (ce n'est pas moi qui gérera le développement)
En tout cas j'ai bien compris grâce à tes explications, merci

[JeanYvette]

J'ai l'impression d'être débile...

J'ai mis votre fonction IS_INJECTION_PATTERN, je peux tester ce que je veux comme paramétre, même 1, ca me renverra toujours 1 et jamais 0... Je ne comprends pas du tout là...

J'ai tenté ca aussi niveau code asp :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
SqlCommand cmd = new SqlCommand();
            cmd.Connection = connection;
            string nom = TextBoxNom.Text;
            cmd.Parameters.AddWithValue("@Nom", nom);
            cmd.Parameters.AddWithValue("@Prenom", TextBoxPrenom.Text.ToString());
            cmd.Parameters.AddWithValue("@NumRue", TextBoxNumRue.Text);
            cmd.Parameters.AddWithValue("@NomRue", TextBoxNomRue.Text.ToString());
            cmd.Parameters.AddWithValue("@Ville", TextBoxVille.Text.ToString());
            cmd.Parameters.AddWithValue("@Identifiant", TextBoxSecuSociale.Text.ToString());
            cmd.Parameters.AddWithValue("@Metier", TextBoxMetier.Text.ToString());
            cmd.CommandText = "exec dbo.InsertionPourPatient @Nom,@Prenom,@NumRue,@NomRue,@Ville,@Identifiant,@Metier";
            cn.setCommand(cmd);
            Response.Write(cmd.Parameters["@Nomrue"].Value);
            //SqlDataReader rdr = cn.ExecuteCommand();
            int a = cn.ExecuteCommands();

Mais ca n'enregistre pas l'utilisateur, et je n'ai aucunes erreur

[StringBuilder]

Vous me faites mal à la tête :o

Code csharp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
SqlCommand cmd = connection.CreateCommand();
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = "dbo.InsertionPourPatient";
cmd.Parameters.AddWithValue("Nom", TextBoxNom.Text);
cmd.Parameters.AddWithValue("Prenom", TextBoxPrenom.Text);
cmd.Parameters.AddWithValue("NumRue", TextBoxNumRue.Text);
cmd.Parameters.AddWithValue("NomRue", TextBoxNomRue.Text);
cmd.Parameters.AddWithValue("Ville", TextBoxVille.Text);
cmd.Parameters.AddWithValue("Identifiant", TextBoxSecuSociale.Text);
cmd.Parameters.AddWithValue("Metier", TextBoxMetier.Text);
cmd.ExecuteNonQuery();

J'ai un doute, il faut peut-être remettre les "@" en préfixe des noms de paramètres, mais je trouve ça étrange... Normalement, pas besoin. Mais sur plusieurs tutos ils sont présents... A tester si ça marche pas !

[JeanYvette]

Bonjour,
En effet c'est un peu plus "sexy", je vais tester.

Après les @ j'ai vu plusieurs tutos avec, je vais voir sans
Mais je pense surtout que c'est ma fonction [IS_INJECTION_PATTERN] qui pose soucis


Bon je confirme le problème vient de ma fonction ici :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
ALTER FUNCTION [dbo].[IS_INJECTION_PATTERN] (@Value NVARCHAR(max))
RETURNS BIT
BEGIN
   IF EXISTS(SELECT *
                FROM   T_EXCLUDE_INJECTION_PATTERN_EIP 
		        WHERE  @Value LIKE '%' + EIP_PATTERN + '%')
      RETURN 1;
   RETURN 0;
END;

Qui renvoit tout le temps 1

[JeanYvette]

En effet je comprends pourquoi ca renvoit tout le temps 1.. Je vois pas trop trop comment le régler pour le coup

[StringBuilder]

Bon, mais sinon, j'ai pas tout compris...

Si vous utilisez des paramètres à vos requêtes, quelles est l'utilité de vérifier les injections ?

Cette vérification n'est utile que si vous utilisez des requêtes avec des valeurs littérales.

Ou si vous faites du SQL dynamique depuis une procédure stockée par exemple.

Vous faites ça ? Si oui, pourquoi ?

[JeanYvette]

Oui j'en fait par moment (c'est pas beau d'ailleurs mais mon tuteur me l'a demandé)
Dans le cadre où je dois faire un rollback sur une table et sur un ou plusieurs id de la table precisée

Ensuite j'utilise TOUT LE TEMPS de procédures ou fonction donc j'ai une procédure par exemple pour insertion d'users, une pour la modification ect... Donc des valeurs littérales

[StringBuilder]

Si vous utilisez le type "StoredProcedure" et que vous passez proprement des paramètres au procédures stockées, non, vous n'avez pas de valeur littérale.

Code sql :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
 
create procedure CreerCompte
(
   @nom varchar(50)
)
as
begin
   insert into compte (nom) values (@nom);
end;

Code sql :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
SqlCommand cmd = connection.CreateCommand();
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = "dbo.CreerCompte";
cmd.Parameters.AddWithValue("@nom", TextBoxNom.Text);
cmd.ExecuteNonQuery();

=> Y'a pas la trace de la moindre valeur littérale.

Vous pouvez passer ce que vous voulez taper ce que vous voulez dans TextBoxNom, il n'y aura jamais le moindre risque d'injection !


Quant à la partie concernant les "rollback", j'ai pas compris. Un exemple ?

[JeanYvette]

Oui je fais comme ça pour les procédures et les paramétrés donc tout va bien

Pour le rollback je vais prendre l'exemple d'un magasin, qui a un nom et une adresse (explosée en plusieurs champs bien entendu)
Le rollback se passe ainsi

Pour l'insertion :

On insert notre magasin de manière tout à fait normal avec une colonne a la fin qui s'appelle flag_magasin (de maniére général flag_<NomTable> qui a par defaut la valeur null.

Si on fait un rollback de cet insert, on va mettre une valeur unique (donc 0) dans cette colonne flag. Le magasin est donc supprimé de manière abstraite (les requetes n'affichant que les magasins dont la valeur de flag est null) mais est gardé dans la bdd pour pouvoir etre restauré si besoin est

Pour le delete ca fonctionne donc de la même maniére, passant le flag_magasin de 0 à null

Pour l'update c'est un peu plus tordu. Imaginons notre notre magasin ainsi

Nom : MonMagasinBrico
NomRue : Rue des lilas
NumRue : 50
IdMagasin : 6
Flag_magasin : null

(C'est bien entendu qu'un exemple). Maintenant imaginons que notre magasin va se renommer, il y a donc un update. L'update se fait de manière normale mais un nouvel enregistrement va se faire il sera de se type :
Nom : MonMagasinBrico
NomRue : rue des lilas
NumRue : 50
IdMagasin : 7
Flag_magasin : 6

En fait on enregistre les données relatif à notre magasin AVANT la mise à jour et le flag_magasin du nouvel enregistrement référence le l'idMagasin de l'enregistrement initial

Cela dans le but de pouvoir, si erreur ou autre il y a eu, pouvoir faire un rollback de cela. (A noter aussi que le type de requete, l'id du champs requete, le nom de l'utilisateur ayant fait la requete, la date précise à laquelle la requete a ete faite ainsi que l'ip d'où la requete a été faite sont enregistrés dans une table de notre bdd)

[StringBuilder]

Ok.

Mais alors je ne fois pas en quoi vous avez besoin de SQL Dynamique pour faire ça !

Vous avez simplement besoin de deux deux procédures :
- Une qui fait un "bête" DELETE dans la table du magasin.
- Une qui fait un "bête" UDPATE dans la table du magagin.

Et de deux trigger "instead of" pour DELETE et un autre "instead of" pour UPDATE avec :
DELETE :
-> Si "flag_magasin" is null, alors on fait un UPDATE du flag.
-> Sinon, on DELETE pour de vrai la ligne (cas de la purge)

UPDATE :
-> INSERT d'une nouvelle ligne avec les valeurs d'origine
-> UPDATE de la ligne d'origine avec les données modifiées

PS : On peut se passer des triggers et mettre leur code dans les procédures de UPDATE et DELETE du magasin, mais le trigger c'est la garantie que le traitement est effectué systématiquement, quel que soit le point d'entrée.

[JeanYvette]

J'ai peut-être mal compris mais par exemple pour l'insert ma procédure de rollback est comme cela :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
 
 
ALTER procedure [dbo].[RollbackInsertionTrue] (@IdErreur int, @NomTable varchar(50))
	as
	DECLARE @SQL VARCHAR(255) ;
	declare @IdTable int;
/* On recupere, en fonction de la table erreur, le nom de la cle primaire de la table */
	declare @NomIdTable varchar(50)
	set @Idtable = (select Id_table from Tables_bdd where Nom_table = @NomTable);
	set @NomIdTable = (select Nom_Id_Table from Tables_bdd where Id_table = @Idtable);
/*alter table centre disable trigger UpdateCentre; */
	SET @SQL = 'update '+@NomTable+' set flag_'+@NomTable+' = 999 where '+@NomIdTable+' = '+CAST(@IdErreur AS VARCHAR(50));
	EXEC (@SQL);

N'est ce pas du sql dynamique ?