›

Ecouter les ainés... ;)

MalaMala Membre, Modérateur
dans Coin canapé & détente #1

Souvent je m'amuse des automatismes presque désuets que m'ont inculqué mes ainés lorsque je débutais et qui me sont restés avec le temps. Et bien voilà  une faille qui m'a fait beaucoup rire...


http://www.macg.co/os-x/2014/02/ssl-une-faille-majeure-dans-ios-et-os-x-80069


 


Spéciale dédicace à  ceux qui n'aiment pas utiliser systématiquement des accolades!


:p   :p   :p   :p   :p   :p


 


Cela me rappelle la fois où un responsable de projet plus expérimenté m'a fait une relecture de code et m'a dit: "Te poses pas de question gamin, quand tu libères une ressource, repasses toujours tes pointeurs à  null !".


Réponses

  • muqaddarmuqaddar Administrateur
    #2

    Je ne mets pas systématiquement des accolades quand il y a une seule ligne dans la condition, sinon on s'en sort plus.


    Et puis ça fait gagner du scrolling. ;)


  • AliGatorAliGator Membre, Modérateur
    #3

    Je ne mets pas systématiquement des accolades quand il y a une seule ligne dans la condition

    Grave erreur.

    Moi je l'impose dans les Coding Guidelines que j'ai mis en place dans ma boite. Déjà  quand tu rajoutes une ligne plus tard et que tu oublies de rajouter l'accolade, ça fait mal, mais en plus le fait de respecter toujours le même standard ça aide à  être rigoureux et à  la lisibilité.
  • AlakAlak Membre
    #4

    Je m'impose depuis peu (deux ou trois mois) de mettre des accolades même pour une seule ligne.


  • LarmeLarme Membre
    #5

    Au départ, je ne supportais pas de ne pas en voir, maintenant, je n'en mets pas quand le test est simple et clair.


     


    Sinon, moi ce qui me choque surtout, c'est le GoTo.


  • yoannyoann Membre
    #6

    L'usage de goto n'est pas totalement débile, ça évite l'imbrication de if à  n'en plus finir et ça réduit la complexité des équation. ça évite également une pratique bien plus crade qui consiste à  mettre plusieurs return dans un bloc (qui pose de gros soucis de maintenance lorsque on doit modifier les actions faites dans tous les cas avant la fin.


    Si vous lisez un peu plus de code C (sécu, audio, réseau...) vous verrez que le goto a des usages cohérent.


  • CéroceCéroce Membre, Modérateur
    février 2014 modifié #7
    Tout à  fait d'accord avec Yoann. Ce qu'il faut éviter avec le goto, ce sont les sauts en arrière, puisque cela crée des boucles "non-structurées". Mais essaie d'écrire le même code sans goto, tu vas devoir imbriquer les tests, c'est tout à  fait illisible.
  • FKDEVFKDEV Membre
    #8


     


    Cela me rappelle la fois où un responsable de projet plus expérimenté m'a fait une relecture de code et m'a dit: "Te poses pas de question gamin, quand tu libères une ressource, repasses toujours tes pointeurs à  null !".




     


    La clé c'est "Te pose pas de questions".


    J'imagine la scène :


    "Là , t'as pas remis ton pointeur à  NULL, faut le faire."


    "C'est pas la peine, je suis dans le destructeur, il va rien se passer après"


    "Te pose pas de questions j'te dis !"


     


    Les bonnes pratiques c'est souvent une question de réflexe.


     




     


    Sinon, moi ce qui me choque surtout, c'est le GoTo.




     


    Aà¯e. Justement j'allais dire que la bonne nouvelle, c'est que personne n'a critiqué le goto.


     


    C'est la différence entre un bon réflexe (repasse toujours les pointeurs à  NULL, ça peut pas faire de mal) et un mauvais préjugé : "N'emploie jamais le goto".


     


    En fait, ce n'est pas un mauvais préjugé en soi, c'est utile quand on démarre la programmation, mais à  un moment donné, il faut grandir et se faire sa propre opinion.


    C'est un peu comme quand tu dis à  un enfant : "Ne parle jamais aux étrangers". C'est utile jusqu'à  un certain âge, après ça devient problèmatique si à  30 ans tu continues à  ne pas parler aux étrangers.


     


    La règle pour le goto, ce serait plutôt : ne jamais l'employer là  où une boucle structurée (while, for) peut le remplacer, autrement dit, ne jamais retourner en arrière dans le flot d'exécution avec un goto.


     


    Dans le cas du code SSL buggé, c'est un cas typique où le goto est adapté : une série d'initialisation de resources où la moindre erreur doit résulter dans la libération des ressources acquises.

  • FKDEVFKDEV Membre
    février 2014 modifié #9

    Le code complet ici :


    http://opensource.apple.com/source/Security/Security-55471/libsecurity_ssl/lib/sslKeyExchange.c


     


    Ce qui me choque dans ce code :


    -parfois il y a des accolades, parfois non.


    -parfois le if est décomposé, parfois non.


     


    par exemple:



    
//Code bien décomposé avec des accolades
    
err = SSLAllocBuffer(&exchangeParams, len);
    
if(err) {
    
goto fail;
    
}
    
(...)
    

    
//code rammassé sans accolades
    
if ((err = SSLHashMD5.update(&hashCtx, &serverRandom)) != 0)
    
goto fail;

    Je pense qu'il aurait fallu utiliser une macro pour la vérification des "err", il y en presque 50 !


    Cela aurait permet de rajouter un log en plus et éviter les copier/coller.


     


    Un truc du style:



    
#define FAIL_IF_ERROR(X) err = X; if (err) { LOG("%s error %d", "X",
    
err); goto fail; }

    D'un autre côté, planquer un goto dans une macro c'est pas top non plus.


     


     


    D'autre part, ce que je trouve choquant, c'est qu'une même variable locale ("err") est à  la fois employée pour stocker des erreurs intermédiaires et pour stocker le résultat de la fonction de vérification. Dans un code qui est censé faire de la sécurité c'est inacceptable.


     


    Je pense qu'il aurait fallu dédié un paramètre de sortie pour le résultat de la validation.


    Cela fait partie de la règle : "ne jamais utiliser la même variable pour dire deux choses de nature différentes"


    Ici err (et donc le retour de la fonction) signifie à  la fois : tous les appels d'API se sont bien passés et la clé est correcte/incorrecte.


    En retour de fonction, on ne sait pas si c'est la clé qui est invalide ou s'il y a eu un autre problème.


     


    Ci-dessous une version avec un paramètre de sortie en plus :



    
static OSStatus
    
SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams,
    
uint8_t *signature, UInt16 signatureLen,
    
bool* resultverify)
    
{
    
if (!resultVerify) return -1;
    

    
*resultVerify = false; //la clé est supposée fausse
    
(...)
    

    

    
err = sslRawVerify(ctx,
    
ctx->peerPubKey,
    
dataToSign,				/* plaintext */
    
dataToSignLen,			/* plaintext length */
    
signature,
    
signatureLen);
    
if(err) {
    
sslErrorLog("SSLDecodeSignedServerKeyExchange: sslRawVerify "
    
"returned %d\n", (int)err);
    
goto fail;
    
}
    
else
    
{
    
*resultVerify = true; // la clé est correcte
    
}
    

    

    
fail:
    
    SSLFreeBuffer(&signedHashes);
    
    SSLFreeBuffer(&hashCtx);
    
    return err;
    

    
}

  • tabliertablier Membre
    #10

    Je ne m'impose pas de mettre des accolades lorsque le if n'a qu'une ligne. Mais, je mets systématiquement les accolades ouvrantes à  la verticale des fermantes correspondantes, et comme tout le monde j'utilise les tabulations pour formater mon code. Pas comme tout le monde, je groupe les déclarations des variables locales en tête des routines ou méthodes et je continue à  mettre des point-virgule. Cela m'est resté de mon apprentissage du C dans les années 88-90 et je trouve cela plus clair à  la lecture.


    A part dans des cas particulier (comme les sauts multiples à  un traitement d'erreur), j'évite les GOTO.


     


    Je me suis formé au C, seul, en utilisant le K&R et en faisant de multiples erreurs!!


  • tplessistplessis Membre
    février 2014 modifié #11


    La clé c'est "Te pose pas de questions".


    J'imagine la scène :


    "Là , t'as pas remis ton pointeur à  NULL, faut le faire."


    "C'est pas la peine, je suis dans le destructeur, il va rien se passer après"


    "Te pose pas de questions j'te dis !"


     


    Les bonnes pratiques c'est souvent une question de réflexe.


     


     


    Aà¯e. Justement j'allais dire que la bonne nouvelle, c'est que personne n'a critiqué le goto.


     


    C'est la différence entre un bon réflexe (repasse toujours les pointeurs à  NULL, ça peut pas faire de mal) et un mauvais préjugé : "N'emploie jamais le goto".


     


    En fait, ce n'est pas un mauvais préjugé en soi, c'est utile quand on démarre la programmation, mais à  un moment donné, il faut grandir et se faire sa propre opinion.


    C'est un peu comme quand tu dis à  un enfant : "Ne parle jamais aux étrangers". C'est utile jusqu'à  un certain âge, après ça devient problèmatique si à  30 ans tu continues à  ne pas parler aux étrangers.


     


    La règle pour le goto, ce serait plutôt : ne jamais l'employer là  où une boucle structurée (while, for) peut le remplacer, autrement dit, ne jamais retourner en arrière dans le flot d'exécution avec un goto.


     


    Dans le cas du code SSL buggé, c'est un cas typique où le goto est adapté : une série d'initialisation de resources où la moindre erreur doit résulter dans la libération des ressources acquises.




     


    Merci pour l'explication, j'étais aussi resté au stade du "goto c'est le mal". :)


  • DrakenDraken Membre
    #12
    Moi j'aime bien les accolades, surtout avec de jolies filles !
  • FKDEVFKDEV Membre
    février 2014 modifié #13

    Moi j'aime bien les accolades, surtout avec de jolies filles !



    Accolade : n. f.

    Geste consistant à  se serrer dans les bras l'un de l'autre en signe d'affection,
    d'amitié et, en particulier, lors d'une remise de décoration,
    de la réception d'un personnage officiel


    Mouais. Chacun son truc...
  • DrakenDraken Membre
    février 2014 modifié #14
    Au lieu de chopper une définition pédante de l'accolade, vas voir la signification de Free Hugs !
  • FKDEVFKDEV Membre
    février 2014 modifié #15
    "Ne parle pas aux étrangers" et "Free hugs" dans le même sujet, bravo.


    "free hugs", c'est à  la fois rigolo et tragique quand on y pense. Pourquoi les câlins auraient-ils besoin d'être gratuits ?

    Edit : non pas qu'ils doivent être payants hein ! C'est juste que gratuit, c'est déjà  trop cher.

    Cadeau.
  • MalaMala Membre, Modérateur
    #16

    C'est beau de vous voir parler de sentiments!  ;D


     




    La clé c'est "Te pose pas de questions".


    J'imagine la scène :


    "Là , t'as pas remis ton pointeur à  NULL, faut le faire."


    "C'est pas la peine, je suis dans le destructeur, il va rien se passer après"


    "Te pose pas de questions j'te dis !"


     


    Les bonnes pratiques c'est souvent une question de réflexe.




    Et quand tu te mets un point d'arrêt sur ta ligne [super dealloc], tu vois immédiatement les objets non libérés. :p


Connectez-vous ou Inscrivez-vous pour répondre.