En termes d' injection SQL , je comprends parfaitement la nécessité de paramétrer un stringparamètre; c'est l'une des plus anciennes astuces du livre. Mais quand peut-il être justifié de ne pas paramétrer un SqlCommand? Certains types de données sont-ils considérés comme «sûrs» pour ne pas être paramétrés?

Par exemple: Je ne me considère nulle part près d' un expert en SQL, mais je ne peux pas penser à des cas où il serait potentiellement vulnérable à l' injection SQL pour accepter un boolou un intet juste concaténer à droite dans la requête.

Mon hypothèse est-elle correcte ou cela pourrait-il potentiellement laisser une énorme faille de sécurité dans mon programme?

Pour plus de précision, cette question est taguée c #qui est une langue fortement typée; quand je dis «paramètre», pensez à quelque chose comme public int Query(int id) .

Je pense que c'est sûr ... techniquement , mais c'est une terrible habitude à prendre. Voulez-vous vraiment écrire des requêtes comme celle-ci?

var sqlCommand = new SqlCommand("SELECT * FROM People WHERE IsAlive = " + isAlive + 
" AND FirstName = @firstName");

sqlCommand.Parameters.AddWithValue("firstName", "Rob");

Cela vous rend également vulnérable dans la situation où un type passe d'un entier à une chaîne (pensez au numéro d'employé qui, malgré son nom, peut contenir des lettres).

Nous avons donc changé le type de EmployeeNumber de intà string, mais avons oublié de mettre à jour nos requêtes SQL. Oups.

Lorsque vous utilisez une plate - forme fortement typé sur un ordinateur que vous contrôlez (comme un serveur web), vous pouvez empêcher l' injection de code pour les requêtes avec seulement bool, DateTimeou des intvaleurs (et d' autres) numérique. Ce qui pose problème, ce sont les problèmes de performances causés par le fait de forcer le serveur SQL à recompiler chaque requête et en l'empêchant d'obtenir de bonnes statistiques sur les requêtes exécutées avec quelle fréquence (ce qui nuit à la gestion du cache).

Mais cette partie "sur un ordinateur que vous contrôlez" est importante, car sinon un utilisateur peut changer le comportement utilisé par le système pour générer des chaînes à partir de ces valeurs pour inclure du texte arbitraire.

J'aime aussi penser à long terme. Que se passe-t-il lorsque la base de code fortement typée ancienne et cassée d'aujourd'hui est portée via la traduction automatique dans le langage dynamique new-hotness, et que vous perdez soudainement la vérification de type, mais que vous n'avez pas encore tous les bons tests unitaires pour le code dynamique ?

Vraiment, il n'y a aucune bonne raison de ne pas utiliser les paramètres de requête pour ces valeurs. C'est la bonne façon de procéder. Continuez et codez en dur les valeurs dans la chaîne SQL alors qu'elles sont vraiment des constantes, mais sinon, pourquoi ne pas simplement utiliser un paramètre? Ce n'est pas comme si c'était dur.

En fin de compte, je n'appellerais pas cela un bogue , en soi, mais j'appellerais cela une odeur : quelque chose qui ne constitue pas un bogue en soi, mais qui est une forte indication que les bogues sont à proximité, ou le seront éventuellement. Un bon code évite de laisser des odeurs, et tout bon outil d'analyse statique le signalera.

J'ajouterai que ce n'est malheureusement pas le genre d'argument que vous pouvez gagner directement. Cela ressemble à une situation où avoir «raison» ne suffit plus, et marcher sur les orteils de vos collègues pour résoudre ce problème par vous-même n'est pas susceptible de promouvoir une bonne dynamique d'équipe; cela pourrait finalement faire plus de mal que cela n'aide. Une meilleure approche dans ce cas peut être de promouvoir l'utilisation d'un outil d'analyse statique. Cela donnerait de la légitimité et de la crédibilité aux efforts visant à revenir en arrière et à corriger le code existant.

Dans certains cas, il EST possible d'effectuer une attaque par injection SQL avec des variables non paramétrées (concaténées) autres que des valeurs de chaîne - voir cet article de Jon: http://codeblog.jonskeet.uk/2014/08/08/the-bobbytables -culture / .

Le fait est que lorsqu'il ToStringest appelé, un fournisseur de culture personnalisé peut transformer un paramètre non-chaîne en sa représentation sous forme de chaîne qui injecte du SQL dans la requête.

Ce n'est pas sûr même pour les types sans chaîne. Utilisez toujours des paramètres. Période.

Considérez l'exemple de code suivant:

var utcNow = DateTime.UtcNow;
var sqlCommand = new SqlCommand("SELECT * FROM People WHERE created_on <= '" + utcNow + "'");

À première vue, le code semble sûr, mais tout change si vous apportez des modifications dans les paramètres régionaux de Windows et ajoutez l'injection au format de date courte:

Le texte de commande qui en résulte ressemble à ceci:

SELECT * FROM People WHERE created_on <= '26.09.2015' OR '1'<>' 21:21:43'

La même chose peut être faite pour le inttype car l'utilisateur peut définir un signe négatif personnalisé qui peut être facilement changé en injection SQL.

On pourrait soutenir que la culture invariante devrait être utilisée au lieu de la culture actuelle, mais j'ai vu des concaténations de chaînes comme celle-ci tant de fois et il est assez facile de passer à côté lors de la concaténation de chaînes avec des objets en utilisant +.

"SELECT * FROM Table1 WHERE Id =" + intVariable.ToString ()

Sécurité
C'est OK.
Les attaquants ne peuvent rien injecter dans votre variable int typée.

Performance
pas OK.

Il est préférable d'utiliser des paramètres, de sorte que la requête sera compilée une fois et mise en cache pour la prochaine utilisation. La prochaine fois, même avec des valeurs de paramètres différentes, la requête est mise en cache et n'a pas besoin d'être compilée dans le serveur de base de données.

Style de codage
Mauvaise pratique.

• Les paramètres sont plus lisibles
• Peut-être que cela vous permet de vous habituer aux requêtes sans paramètres, puis peut-être avez-vous fait une erreur une fois et utilisez une valeur de chaîne de cette façon et ensuite vous devriez probablement dire au revoir à vos données. Mauvaise habitude!

"SELECT * FROM Produit WHERE Id =" + TextBox1.Text

Bien que ce ne soit pas votre question, mais peut-être utile pour les futurs lecteurs:

Désastre de sécurité !
Même lorsque le Idchamp est un entier, votre requête peut être soumise à une injection SQL. Supposons que vous ayez une requête dans votre application "SELECT * FROM Table1 WHERE Id=" + TextBox1.Text, un attaquant peut insérer dans la zone de texte 1; DELETE Table1et la requête sera:

"SELECT * FROM Table1 WHERE Id=1; DELETE Table1"

Si vous ne souhaitez pas utiliser de requête paramétrée ici, vous devez utiliser des valeurs typées:

string.Format("SELECT * FROM Table1 WHERE Id={0}", int.Parse(TextBox1.Text))

Ta question

Ma question s'est posée parce qu'un collègue a écrit un tas de requêtes concaténant des valeurs entières, et je me demandais si ce n'était pas une perte de temps de toutes les résoudre.

Je pense que changer ces codes n'est pas une perte de temps. En effet, le changement est recommandé!

si votre collègue utilise des variables int, il n'a aucun risque pour la sécurité, mais je pense que changer ces codes n'est pas une perte de temps et en effet, il est recommandé de changer ces codes. Cela rend le code plus lisible, plus maintenable et accélère l'exécution.

Il y a en fait deux questions en une. Et la question du titre a très peu à voir avec les préoccupations exprimées par le PO dans les commentaires qui suivent.

Bien que je réalise que pour l'OP, c'est leur cas particulier qui compte, pour les lecteurs venant de Google, il est important de répondre à la question plus générale, qui peut être formulée comme "est la concaténation aussi sûre que les déclarations préparées si je me suis assuré que chaque littéral que je concatène est sûr? ". Alors, je voudrais me concentrer sur ce dernier. Et la réponse est

Définitivement non.

L'explication n'est pas aussi directe que la plupart des lecteurs le souhaiteraient, mais je ferai de mon mieux.

Je réfléchis à la question depuis un moment, ce qui a abouti à l' article (bien que basé sur l'environnement PHP) où j'ai essayé de tout résumer. Il m'est venu à l'esprit que la question de la protection contre l'injection SQL échappe souvent à certains sujets connexes mais plus étroits, comme l'échappement de chaîne, la conversion de type, etc. Bien que certaines des mesures puissent être considérées comme sûres lorsqu'elles sont prises par elles-mêmes, il n'y a pas de système, ni de règle simple à suivre. Ce qui rend le terrain très glissant, mettant trop l'accent sur l'attention et l'expérience du développeur.

La question de l'injection SQL ne peut pas être simplifiée à une question de syntaxe particulière. C'est plus large que la moyenne des développeurs de penser. C'est aussi une question méthodologique . Il ne s'agit pas seulement de «quelle mise en forme particulière nous devons appliquer», mais aussi de « comment cela doit être fait».

(De ce point de vue, un article de Jon Skeet cité dans l'autre réponse fait plutôt mal que bien, car il est à nouveau pinailleur sur un cas particulier, se concentrant sur un problème de syntaxe particulier et ne parvenant pas à résoudre le problème dans son ensemble.)

Lorsque vous essayez d'aborder la question de la protection non pas dans son ensemble mais comme un ensemble de problèmes de syntaxe différents, vous êtes confronté à une multitude de problèmes.

• la liste des choix de formatage possibles est vraiment énorme. Cela signifie que l'on peut facilement en oublier. Ou confondez-les (en utilisant une chaîne d' échappement pour l' identifiant par exemple).
• La concaténation signifie que toutes les mesures de protection doivent être effectuées par le programmeur et non par le programme. Ce problème à lui seul entraîne plusieurs conséquences:
  • un tel formatage est manuel. Manuel signifie extrêmement sujet aux erreurs. On pourrait simplement oublier de postuler.
  • de plus, il y a une tentation de déplacer les procédures de formatage dans une fonction centralisée, ce qui dérange encore plus les choses et gâche les données qui ne vont pas dans la base de données.
• lorsque plus d'un développeur est impliqué, les problèmes se multiplient par dix.
• lorsque la concaténation est utilisée, on ne peut pas dire en un coup d'œil une requête potentiellement dangereuse: elles sont toutes potentiellement dangereuses!

Contrairement à ce désordre, les déclarations préparées sont en effet le Saint Graal:

• il peut être exprimé sous la forme d'une règle simple et facile à suivre.
• il s'agit essentiellement d'une mesure indétachable, ce qui signifie que le développeur ne peut pas interférer et, volontairement ou non, gâcher le processus.
• la protection contre l'injection n'est en réalité qu'un effet secondaire des instructions préparées, dont le but réel est de produire une instruction syntaxiquement correcte. Et une déclaration syntaxiquement correcte est 100% preuve d'injection. Pourtant, nous avons besoin que notre syntaxe soit correcte malgré toute possibilité d'injection.
• s'il est utilisé sur tout le pourtour, il protège l'application quelle que soit l'expérience du développeur. Dites, il y a une chose appelée injection de second ordre . Et une très forte illusion qui se lit "afin de protéger, évitez toutes les entrées fournies par l'utilisateur ". Combinées ensemble, elles conduisent à l'injection, si un développeur prend la liberté de décider, ce qui doit être protégé et ce qui ne l'est pas.

(En réfléchissant plus loin, j'ai découvert que l'ensemble actuel d'espaces réservés n'est pas suffisant pour les besoins réels et doit être étendu, à la fois pour les structures de données complexes, comme les tableaux, et même les mots-clés ou identifiants SQL, qui doivent parfois être ajoutés au requête dynamiquement aussi, mais un développeur n'est pas armé pour un tel cas, et obligé de revenir à la concaténation de chaînes, mais c'est une autre question).

Fait intéressant, la controverse de cette question est provoquée par la nature très controversée de Stack Overflow. L'idée du site est d'utiliser des questions particulières d'utilisateurs qui les demandent directement pour atteindre l'objectif d'avoir une base de données de réponses à usage général adaptée aux utilisateurs issus de la recherche . L'idée n'est pas mauvaise en soi , mais elle échoue dans une situation comme celle-ci: lorsqu'un utilisateur pose une question très étroite , notamment pour obtenir une dispute dans un différend avec un collègue (ou pour décider s'il vaut la peine de refactoriser le code). Alors que la plupart des participants expérimentés essaient d'écrire une réponse, en gardant à l'esprit la mission de Stack Overflow dans son ensemble, ce qui rend leur réponse bonne pour autant de lecteurs que possible, pas seulement pour l'OP.

Ne pensons pas seulement aux considérations de sécurité ou de type sécurisé.

La raison pour laquelle vous utilisez des requêtes paramétrées est d'améliorer les performances au niveau de la base de données. Du point de vue d'une base de données, une requête paramétrée est une requête dans le tampon SQL (pour utiliser la terminologie d'Oracle bien que j'imagine que toutes les bases de données ont un concept similaire en interne). Ainsi, la base de données peut contenir un certain nombre de requêtes en mémoire, préparées et prêtes à être exécutées. Ces requêtes n'ont pas besoin d'être analysées et seront plus rapides. Les requêtes fréquemment exécutées seront généralement dans la mémoire tampon et n'auront pas besoin d'être analysées à chaque fois qu'elles sont utilisées.

SAUF SI

Quelqu'un n'utilise pas de requêtes paramétrées. Dans ce cas, la mémoire tampon est continuellement vidée par un flux de requêtes presque identiques dont chacune doit être analysée et exécutée par le moteur de base de données et les performances en souffrent tous les jours car même les requêtes fréquemment exécutées finissent par être ré-analysées plusieurs fois journée. J'ai mis au point des bases de données pour gagner ma vie et cela a été l'une des plus grandes sources de fruits à portée de main.

MAINTENANT

Pour répondre à votre question, si votre requête comporte un petit nombre de valeurs numériques distinctes, vous ne causerez probablement pas de problèmes et pourrez en fait améliorer les performances à l'infini. SI cependant il y a potentiellement des centaines de valeurs et que la requête est souvent appelée, vous allez affecter les performances de votre système alors ne le faites pas.

Oui, vous pouvez augmenter la mémoire tampon SQL, mais c'est toujours au final au détriment d'autres utilisations plus critiques de la mémoire comme la mise en cache des index ou des données. Moral, utilisez des requêtes paramétrées assez religieusement pour optimiser votre base de données et utiliser plus de mémoire serveur pour les choses qui comptent ...

Pour ajouter des informations à la réponse Maciek:

Il est facile de modifier les informations de culture d'une application tierce .NET en appelant la fonction principale de l'assembly par réflexion:

using System;
using System.Globalization;
using System.Reflection;
using System.Threading;

namespace ConsoleApplication2
{
  class Program
  {
    static void Main(string[] args)
    {
      Assembly asm = Assembly.LoadFile(@"C:\BobbysApp.exe");
      MethodInfo mi = asm.GetType("Test").GetMethod("Main");
      mi.Invoke(null, null);
      Console.ReadLine();
    }

    static Program()
    {
      InstallBobbyTablesCulture();
    }

    static void InstallBobbyTablesCulture()
    {
      CultureInfo bobby = (CultureInfo)CultureInfo.InvariantCulture.Clone();
      bobby.DateTimeFormat.ShortDatePattern = @"yyyy-MM-dd'' OR ' '=''";
      bobby.DateTimeFormat.LongTimePattern = "";
      bobby.NumberFormat.NegativeSign = "1 OR 1=1 OR 1=";
      Thread.CurrentThread.CurrentCulture = bobby;
    }
  }
}

Cela ne fonctionne que si la fonction principale de BobbysApp est publique. Si Main n'est pas publique, vous pouvez appeler d'autres fonctions publiques.

À mon avis, si vous pouvez garantir que le paramètre avec lequel vous travaillez ne contiendra jamais de chaîne, il est sûr mais je ne le ferais en aucun cas. En outre, vous constaterez une légère baisse des performances en raison du fait que vous effectuez une concaténation. La question que je voudrais vous poser est la suivante: pourquoi ne voulez-vous pas utiliser de paramètres?

C'est correct mais jamais sûr .. et la sécurité dépend toujours des entrées, par exemple si l'objet d'entrée est TextBox, les attaquants peuvent faire quelque chose de délicat car la zone de texte peut accepter la chaîne, vous devez donc mettre une sorte de validation / conversion pour être en mesure d'éviter aux utilisateurs la mauvaise entrée. Mais le fait est que ce n'est pas sûr. Aussi simplement que cela.

Non, vous pouvez obtenir une attaque par injection SQL de cette façon. J'ai écrit un vieil article en turc qui montre comment ici . Exemple d'article en PHP et MySQL mais le concept fonctionne de la même manière en C # et SQL Server.

Fondamentalement, vous attaquez de la manière suivante. Considérons que vous avez une page qui affiche des informations en fonction de la valeur de l'identifiant entier. Vous ne l'avez pas paramétré en valeur, comme ci-dessous.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=24

D'accord, je suppose que vous utilisez MySQL et j'attaque de la manière suivante.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=ASCII((SELECT%20DATABASE()))

Notez qu'ici la valeur injectée n'est pas une chaîne. Nous changeons la valeur de char en int en utilisant la fonction ASCII. Vous pouvez accomplir la même chose dans SQL Server en utilisant "CAST (YourVarcharCol AS INT)".

Après cela, j'utilise les fonctions de longueur et de sous-chaîne pour trouver le nom de votre base de données.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=LEN((SELECT%20DATABASE()))

http://localhost/sqlEnjeksiyon//instructors.aspx?id=ASCII(SUBSTR(SELECT%20DATABASE(),1,1))

Ensuite, en utilisant le nom de la base de données, vous commencez à obtenir les noms de table dans la base de données.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=ASCII(SUBSTR((SELECT table_name FROM INFORMATION_SCHEMA.TABLES LIMIT 1),1,1))

Bien sûr, vous devez automatiser ce processus, car vous n'obtenez qu'un seul caractère par requête. Mais vous pouvez facilement l'automatiser. Mon article montre un exemple en watir . Utilisation d'une seule page et non d'une valeur d'ID paramétrée. Je peux apprendre chaque nom de table dans votre base de données. Après cela, je peux rechercher des tableaux importants. Cela prendra du temps, mais c'est faisable.

Eh bien ... une chose est sûre: la sécurité n'est PAS ok, lorsque vous concaténez une chaîne (prise par l'utilisateur) avec votre chaîne de commande SQL. Cela n'a pas d'importance chaque fois que la clause where fait référence à un entier ou à n'importe quel type; des injections pourraient se produire.

Ce qui compte dans l'injection SQL, c'est le type de données de la variable utilisée pour obtenir la valeur de l'utilisateur.

Supposons que nous ayons un entier dans la clause where et:

1. la variable utilisateur est une chaîne. Alors ok, ce n'est pas très facile à injecter (avec UNION) mais il est très facile de contourner en utilisant des attaques de type 'OR 1 = 1' ...

2. Si la variable utilisateur est un entier. Ensuite, nous pouvons `` tester '' la force du système en passant des tests de gros nombres inhabituels pour les pannes du système ou même pour un dépassement de tampon caché (sur la chaîne finale) ...;)

Peut-être que les paramètres des requêtes ou (encore mieux - imo) des procédures stockées ne sont pas 100% sûrs contre les menaces, mais ils sont la mesure la moins requise (ou l'élémentaire si vous préférez) pour les minimiser.