Comment puis-je amener les programmeurs à arrêter d'écrire du code vulnérable à l'injection SQL?

11

Parfois, vous êtes occupé et déléguez de petites tâches à des programmeurs débutants. Mais si vous n'y prêtez pas suffisamment attention, vous vous retrouvez avec ce type de code en production:

class DivtoggleController extends Zend_Controller_Action {

    public function closeAction() {
        /* ... code removed for brevity ... */

        $req = $this->getRequest();
        $formData = $req->getPost();

        $d = $formData['div'];
        $i = $formData['id'];

        $dm = new Model_DivtoggleManager();
        $rs = $dm->setDivToggleById($d, $i);

    }

}


class Model_DivtoggleManager extends Zend_Db_Table {

    public function setDivToggleById($div, $id) {
        $result = $this->getAdapter()->query(
           "update div_toggle set " . $div . "=1 where id=" . $id
        );
    }

}

Donc, étant donné que j'ai supprimé la logique de gestion de l'authentification / session par souci de concision, qui peut me dire quel problème possible il pourrait y avoir avec cet exemple?

Roger Halliburton
la source
Pourquoi cela n'est-il pas stocké dans un simple cookie?
Darknight
1
@Darknight, vous supposez qu'il existe une session pour stocker le cookie. Qu'il y ait ou non est sans rapport avec le problème de sécurité plus large.
Roger Halliburton

Réponses:

24

Vous pouvez leur apprendre. Tout le monde le fait au début, même vous. Si ce type de code entre en production, c'est la faute des personnes âgées; pas le junior.

Éditer:

L'une des choses que j'ai faites est que j'ai personnellement pris la décision de demander aux gens de réviser mon code (y compris les juniors) avant une version. Le code est révisé, les juniors le voient comme une expérience d'apprentissage, les gens perdent la peur de la révision du code comme punition et commencent à faire la même chose.

John Kraft
la source
2
+1 Je dois être d'accord. Vous ne pouvez pas blâmer un programmeur junior (probablement) parce que vous avez supposé un niveau de connaissances qu'il ne possède peut-être pas. (Cela dit, vous auriez aimé penser ces problèmes sont bien connus dans ce jour et l' âge Là encore, je. Aime penser que je suis doué et beau, etc.) :-)
John Parker
Une déclaration assez juste. Je suppose que je devrais poser une question de suivi demandant pourquoi la direction insisterait pour lancer du code en production qui n'a pas été approuvé par les programmeurs seniors. Dois-je risquer mon travail pour un problème de sécurité mineur?
Roger Halliburton
1
@Roger Halliburton: Eh bien, si ce problème de sécurité est exploité, quel est le pire qui pourrait arriver? Et pourquoi signaler des problèmes de sécurité vous coûterait-il votre travail? La direction ne veut-elle pas que cela soit réglé?
FrustratedWithFormsDesigner
1
@Roger - c'est seulement mineur jusqu'à ce que vous soyez piraté. :) Je pense que mettre du code en production sans examen (quel que soit le niveau du développeur) est un échec. Malheureusement, c'est une pratique très courante dans de nombreuses entreprises; et, ime, il faut généralement une grosse somme de $ 4! t avant que la direction commence à évaluer des choses comme les revues de code.
John Kraft
1
@Roger: Tout le code doit être révisé, pas seulement le code écrit par des programmeurs "juniors". Même les programmeurs seniors font des erreurs.
Dean Harding le
20

Piratez leur code devant leurs yeux puis montrez-leur comment le corriger. Encore et encore jusqu'à ce qu'ils comprennent.

Joe Phillips
la source
4
Envoyez-leur un e-mail tous les matins: "Au fait, j'ai de nouveau supprimé votre base de données hier soir via une autre vulnérabilité d'injection SQL que vous avez laissée dans votre code. Je sais combien vous aimez restaurer les sauvegardes de base de données!: D"
Ant
2
@Ant: Soyez gentil. Faites-le peu de temps après la sauvegarde planifiée, pas peu de temps avant.
Donal Fellows
@Donal: faites-le peu de temps après la sauvegarde, puis dites-leur que la sauvegarde a échoué et laissez-les transpirer le reste de la journée avant de restaurer la sauvegarde pendant la nuit.
jwenting
8

Vous pouvez les obliger à suivre un cours dès qu'ils rejoignent votre entreprise, avant d'avoir accès au contrôle de code source, ce qui les initie aux injections SQL, aux scripts intersites, à la falsification de requêtes intersites et à d'autres vulnérabilités courantes. Couvrez les exemples en face à face, cassez le mauvais code devant eux, faites-les casser le mauvais code et dirigez-les vers le site OWASP pour plus d'informations une fois qu'ils auront obtenu leur diplôme.

Vous pouvez également exiger l'utilisation d'une bibliothèque personnalisée qui gère cela pour vous, mais ce n'est qu'une solution secondaire car ils seront sûrs d'exécuter des requêtes personnalisées lorsque cela deviendra plus pratique.

Si vous avez les ressources, il peut également être utile de s'assurer que les membres les plus expérimentés de l'équipe vérifient leurs différences avant de s'engager.

La connaissance, c'est le pouvoir!

yan
la source
3
Il est préférable de les éliminer avant de rejoindre votre entreprise. Si vous embauchez quelqu'un pour écrire quoi que ce soit qui utilise SQL, ne posez pas de questions d'entrevue sur les limites d'injection sur l'incompétence.
Wooble
Je serais généralement d'accord, mais une location junior très intelligente et ambitieuse est beaucoup, beaucoup moins chère qu'un "développeur PHP avec N années d'expérience", ce qui n'est pas garanti d'être beaucoup mieux. Les développeurs juniors intelligents peuvent ramasser ces trucs très rapidement et être un atout.
yan
3

En supposant que c'est l'insécurité à laquelle d'autres ont fait référence, en tant que développeur de tout niveau, il est facile d'oublier que getPost () ne sécurise pas les données en premier.

Une solution consiste à:

  1. Écrivez une classe qui obtient toutes les données POST / GET et les écrit telles quelles dans une classe Singleton appelée «insecure_data». Désactivez ensuite les tableaux POST / GET.
  2. Les développeurs doivent récupérer les données POST / GET à partir du tableau «insecure_data», et non les tableaux POST / GET.

Tout développeur qui récupère quelque chose dans un tableau appelé «insecure_data» et ne prend pas la peine de le sécuriser est soit ignorant, soit paresseux. Si c'est le premier, offrez une formation, après quoi ce doit être le second - et alors vous avez un problème disciplinaire, pas un problème de programmation.

Dan souffle
la source
Wow, c'est compliqué inutilement et ne résout toujours pas le problème. Il ne s'agit pas de nettoyer l'entrée juste pour le plaisir, mais de nettoyer les données que vous placez dans la base de données, et ces données pourraient théoriquement provenir de n'importe où, d'un formulaire Web, d'un e-mail ou d'un document XML qui quelqu'un télécharge. Dans tous ces cas, vous devez effectuer un nettoyage lors de l'accès à la base de données.
Roger Halliburton
@RogerHalliburton Bien sûr , il ne résout pas tout, mais il est un concept (une sorte de modèle de conception, si vous voulez) plutôt qu'une mesure de sécurité à entièrement étoffée pour rendre les données non sécurisé regard d' insécurité.
Dan Blows
Ah, je vois. Mais vous vous battez contre la nature humaine, si vous dites à quelqu'un "Cet objet n'est pas sûr à moins qu'il ne soit marqué comme sécurisé" et qu'il essaie de l'utiliser pour quelque chose, la plupart du temps il le signalera comme sécurisé sans vraiment le vérifier. Eh bien, profitez de l'amélioration de la réputation.
Roger Halliburton
Du point de vue d'un développeur débutant, le simple fait de voir $ insecure_data lève un drapeau d'une manière différente du simple fait de voir $ postdata (ou autre). L'idée vient de Joel Spolsky, "Faites en sorte que le mauvais code paraisse mal" . Si la nature humaine de vos développeurs est telle d'ignorer ce drapeau rouge, alors vous devez soit fournir beaucoup de formation ou obtenir de nouveaux développeurs.
Dan Blows
Je ne garde pas Spolsky sur un piédestal. Ce sont des développeurs qui ignorent facilement les tabulations incohérentes, ils ne vont pas réfléchir à deux fois avant d'utiliser quelque chose qui est appelé "non sécurisé".
Roger Halliburton
1

L'un des meilleurs guides que j'ai lu sur la sécurité Web est ce guide de sécurité Ruby on Rails . Bien qu'il s'agisse de Ruby on Rails, de nombreux concepts s'appliquent à tout développement Web. J'encourage toute personne nouvelle à lire ce guide.

mistagrooves
la source
2
Tout le monde sait que Ruby n'est pas sécurisé.
Roger Halliburton
5
@Roger: «Tout le monde sait» toutes sortes de choses qui pourraient ou non être vraies. Je préconise une approche de la programmation fondée sur la réalité, et non pas une approche par ouï-dire.
Donal Fellows
0

Le code que vous avez lié ci-dessus est susceptible d'une attaque par injection SQL, car les entrées HTTP que vous utilisez dans la requête n'ont pas été nettoyées avec mysql_real_escape_stringou tout autre moyen.

Ryan
la source
1
oh, je pensais que nous répondions à cela comme une question test:]
Ryan
Les votes descendants sont un peu boiteux, car le libellé de la question a été modifié ex post: P
Ryan
0

En ce qui concerne votre question (probablement prioritaire) "comment puis-je amener les programmeurs à cesser de faire cela", je dirais que les encadrer régulièrement, expliquer soigneusement le problème en question (et les retombées potentielles, etc.) et mettre l'accent sur l'importance des vulnérabilités du code (à la fois en termes d'injection SQL et de script intersite, etc.) est probablement la solution la plus judicieuse.

S'ils continuent de gâcher malgré tout ce qui précède (vous voudrez peut-être jeter un œil sur leurs engagements, etc. plutôt que de découvrir "en direct"), alors le problème est soit que vous leur échouez en tant que mentor, soit que ils ont peut-être besoin de trouver quelque chose de plus approprié pour vivre.

John Parker
la source