Quel est le code plus mal que vous avez jamais vu dans un environnement d’entreprise de production ?

Question

Ce qui est le plus mal ou dangereux fragment de code que vous avez jamais vu dans un environnement de production au sein d'une entreprise? Je n'ai jamais rencontré de production de code que je considère être délibérément malveillantes et le mal, donc je suis assez curieux de voir ce que les autres ont trouvé.

Le plus dangereux de code que j'ai jamais vu, c'était une procédure stockée deux liées aux serveurs de loin de notre base de production de serveur de base de données. La procédure stockée accepté de type NVARCHAR(8000) paramètre et exécuté le paramètre sur la cible serveur de production via un double-saut sp_executeSQL de commande. C'est-à-dire, la sp_executeSQL commande exécutée une autre sp_executeSQL commande dans l'ordre de sauter deux serveurs liés. Oh, et le compte du serveur lié avaient des droits d'administrateur système sur la cible serveur de production.

Answer 1

Avertissement: Long effrayant post à venir

J'ai écrit sur une application, j'ai travaillé sur l'avant ici et ici. Pour faire simple, ma société a hérité de 130 000 lignes de déchets en provenance de l'Inde. L'application est écrite en C#; c'est un guichet d'application, le même type de logiciel scrutateurs utilisation derrière le comptoir, à chaque fois que vous allez à la banque. L'application s'est écrasé 40 à 50 fois par jour, et il ne pouvait tout simplement pas être remaniée dans le code du travail. Mon entreprise a dû ré-écrire la totalité de l'app au cours de 12 mois.

Pourquoi cette application est le mal? Parce que la vue du code source était suffisant pour un homme sain d'esprit mad et mad homme sain d'esprit. La logique tordue utilisé pour écrire cette application pourrait avoir été inspirée par un Lovecraftian cauchemar. Caractéristiques uniques de cette application inclus:

• Hors de 130 000 lignes de code, l'application entière contenue 5 classes (à l'exclusion des fichiers de formulaire). L'ensemble de ces publics avaient été les classes statiques. Une classe a été appelé Globals.cs, qui contenait 1000 et 1000 et 1000 des publics les variables statiques sont utilisés pour contenir l'ensemble de l'état de l'application. Ces cinq classes contenues 20 000 lignes de code au total, avec le reste du code embarqué dans les formes.

• Vous devez vous demander, comment les programmeurs d'écrire une grosse application sans classes? Qu'ont-ils utiliser pour représenter leurs objets de données? Il s'avère que les programmeurs a réussi à ré-inventer la moitié des concepts que nous avons tous appris à propos de la programmation orientée objet, simplement en combinant ArrayLists, tables de hachage, et les tables de données. Nous avons vu beaucoup de ceci:

  • ArrayLists de tables de hachage
  • Les tables de hashage avec de la ficelle de clés et de valeurs DataRow
  • ArrayLists de DataTables
  • Des datarow contenant ArrayLists qui contenait les tables de hashage
  • ArrayLists de des datarow
  • ArrayLists de ArrayLists
  • Les tables de hashage avec de la ficelle des clés et des valeurs de la table de hachage
  • ArrayLists de ArrayLists de tables de hachage
  • Toute autre combinaison de ArrayLists, tables de hachage, les tables de données que vous pouvez penser.

  Gardez à l'esprit, aucune des structures de données ci-dessus sont fortement typés, de sorte que vous avez à jeter tout ce mystère de l'objet que vous sortez de la liste pour le bon type. C'est incroyable ce genre de complexe, de Rube Goldberg-comme des structures de données que vous pouvez créer en utilisant simplement ArrayLists, tables de hachage, et les tables de données.

• Pour donner un exemple de comment utiliser le modèle d'objet détaillé ci-dessus, examiner les Comptes: le programmeur original créé une autre table de hachage pour chaque concievable propriété d'un compte: une table de hachage appelé hstAcctExists, hstAcctNeedsOverride, hstAcctFirstName. Les clés pour l'ensemble de ces tables de hachage a été un "|" chaîne séparée. Concevable clés inclus "123456|DDA", "24100|SVG", "100|LNS", etc.

• Étant donné que l'état de l'ensemble de l'application est facilement accessible à partir de variables globales, les programmeurs trouvé inutile de passer des paramètres aux méthodes. Je dirais que 90% des méthodes a 0 paramètres. De la quelques qui ne, tous les paramètres ont été transmis comme des chaînes de caractères pour plus de commodité, indépendamment de ce que la chaîne représentée.

• Sans effets secondaires fonctions n'existent pas. Chaque méthode modifiée de 1 ou plusieurs variables dans la classe Globals. Pas tous les effets secondaires de sens; par exemple, l'une des formes les méthodes de validation avait un côté mystérieux effet de calcul de fil et de court-paiements sur les prêts pour tout ce qui compte a été stocké Globals.lngAcctNum.

• Bien qu'il existe beaucoup de formes, il y a un formulaire pour les gouverner tous: frmMain.cs, qui contenait une bagatelle de 20 000 lignes de code. Ce n'frmMain faire? Tout. Il leva les comptes, les reçus imprimés, distribués en espèces, il a tout fait.

  Parfois, d'autres formulaires nécessaires pour l'appel de méthodes sur frmMain. Plutôt que de facteur de code du formulaire dans une salle de classe, pourquoi ne pas simplement appeler directement le code:

  ((frmMain)this.MDIParent).UpdateStatusBar(hstValues);
  

• Pour chercher des comptes, les programmeurs ont fait quelque chose comme ceci:

  bool blnAccountExists =
      new frmAccounts().GetAccountInfo().blnAccountExists
  

  Aussi mauvaise qu'elle l'est déjà de la création d'un formulaire invisible pour effectuer une logique d'entreprise, comment pensez-vous que la forme savait ce qui compte pour regarder en haut? C'est simple: le formulaire d'accès Globals.lngAcctNum et Globales.strAcctType. (Qui n'aime pas la notation hongroise?)

• Réutiliser le Code était synonyme de ctrl-c, ctrl-v. j'ai trouvé 200-les méthodes en ligne de copie/collé sur 20 formes.

• L'application avait un étrange modèle de thread, quelque chose que j'aime appeler le fil-et-minuterie modèle: chaque forme qui a donné naissance à un fil avait un timer sur elle. Chaque thread qui a vu le jour, a lancé une minuterie qui avait un 200 ms de retard; une fois que la minuterie a commencé, il va vérifier pour voir si le fil avait mis un peu de magie booléen, il abandonne le fil. Le résultant ThreadAbortException a été avalé.

  Vous pensez que vous ne voyez ce modèle une fois, mais je l'ai trouvé dans au moins 10 des endroits différents.

• En parlant de threads, le mot "lock" n'est jamais apparu dans l'application. Fils manipulé de l'état global librement sans prendre une serrure.

• Chaque méthode dans la demande contenait un bloc try/catch. Chaque exception a été enregistré et d'ingestion.

• Qui a besoin de basculer sur les enums lors de la commutation sur les cordes est tout aussi facile!

• Certaines génie compris que vous pouvez brancher plusieurs contrôles de formulaire à la même gestionnaire d'événements. Comment le programmeur de gérer cela?

  private void OperationButton_Click(object sender, EventArgs e)
  {
      Button btn = (Button)sender;
      if (blnModeIsAddMc)
      {
          AddMcOperationKeyPress(btn);
      }
      else
      {
          string strToBeAppendedLater = string.Empty;
          if (btn.Name != "btnBS")
          {
              UpdateText();
          }
          if (txtEdit.Text.Trim() != "Error")
          {
              SaveFormState();
          }
          switch (btn.Name)
          {
              case "btnC":
                  ResetValues();
                  break;
              case "btnCE":
                  txtEdit.Text = "0";
                  break;
              case "btnBS":
                  if (!blnStartedNew)
                  {
                      string EditText = txtEdit.Text.Substring(0, txtEdit.Text.Length - 1);
                      DisplayValue((EditText == string.Empty) ? "0" : EditText);
                  }
                  break;
              case "btnPercent":
                  blnAfterOp = true;
                  if (GetValueDecimal(txtEdit.Text, out decCurrValue))
                  {
                      AddToTape(GetValueString(decCurrValue), (string)btn.Text, true, false);
                      decCurrValue = decResultValue * decCurrValue / intFormatFactor;
                      DisplayValue(GetValueString(decCurrValue));
                      AddToTape(GetValueString(decCurrValue), string.Empty, true, false);
                      strToBeAppendedLater = GetValueString(decResultValue).PadLeft(20)
                                                  + strOpPressed.PadRight(3);
                      if (arrLstTapeHist.Count == 0)
                      {
                          arrLstTapeHist.Add(strToBeAppendedLater);
                      }
                      blnEqualOccurred = false;
                      blnStartedNew = true;
                  }
                  break;
              case "btnAdd":
              case "btnSubtract":
              case "btnMultiply":
              case "btnDivide":
                  blnAfterOp = true;
                  if (txtEdit.Text.Trim() == "Error")
                  {
                      btnC.PerformClick();
                      return;
                  }
                  if (blnNumPressed || blnEqualOccurred)
                  {
                      if (GetValueDecimal(txtEdit.Text, out decCurrValue))
                      {
                          if (Operation())
                          {
                              AddToTape(GetValueString(decCurrValue), (string)btn.Text, true, true);
                              DisplayValue(GetValueString(decResultValue));
                          }
                          else
                          {
                              AddToTape(GetValueString(decCurrValue), (string)btn.Text, true, true);
                              DisplayValue("Error");
                          }
                          strOpPressed = btn.Text;
                          blnEqualOccurred = false;
                          blnNumPressed = false;
                      }
                  }
                  else
                  {
                      strOpPressed = btn.Text;
                      AddToTape(GetValueString(0), (string)btn.Text, false, false);
                  }
                  if (txtEdit.Text.Trim() == "Error")
                  {
                      AddToTape("Error", string.Empty, true, true);
                      btnC.PerformClick();
                      txtEdit.Text = "Error";
                  }
                  break;
              case "btnEqual":
                  blnAfterOp = false;
                  if (strOpPressed != string.Empty || strPrevOp != string.Empty)
                  {
                      if (GetValueDecimal(txtEdit.Text, out decCurrValue))
                      {
                          if (OperationEqual())
                          {
                              DisplayValue(GetValueString(decResultValue));
                          }
                          else
                          {
                              DisplayValue("Error");
                          }
                          if (!blnEqualOccurred)
                          {
                              strPrevOp = strOpPressed;
                              decHistValue = decCurrValue;
                              blnNumPressed = false;
                              blnEqualOccurred = true;
                          }
                          strOpPressed = string.Empty;
                      }
                  }
                  break;
              case "btnSign":
                  GetValueDecimal(txtEdit.Text, out decCurrValue);
                  DisplayValue(GetValueString(-1 * decCurrValue));
                  break;
          }
      }
  }
  

• Le même génie aussi découvert la glorieuse opérateur ternaire. Voici quelques exemples de code:

  frmTranHist.cs [line 812]:

  strDrCr = chkCredits.Checked && chkDebits.Checked ? string.Empty
                      : chkDebits.Checked ? "D"
                          : chkCredits.Checked ? "C"
                              : "N";
  

  frmTellTransHist.cs [line 961]:

  if (strDefaultVals == strNowVals && (dsTranHist == null ? true : dsTranHist.Tables.Count == 0 ? true : dsTranHist.Tables[0].Rows.Count == 0 ? true : false))
  

  frmMain.TellCash.cs [line 727]:

  if (Validations(parPostMode == "ADD" ? true : false))
  

• Voici un extrait de code qui illustre le phénomène classique de la mauvaise utilisation de l'objet StringBuilder. Notez comment le programmeur concats une chaîne de caractères dans une boucle, puis ajoute la chaîne résultante à la StringBuilder:

  private string CreateGridString()
  {
      string strTemp = string.Empty;
      StringBuilder strBuild = new StringBuilder();
      foreach (DataGridViewRow dgrRow in dgvAcctHist.Rows)
      {
          strTemp = ((DataRowView)dgrRow.DataBoundItem)["Hst_chknum"].ToString().PadLeft(8, ' ');
          strTemp += "  ";
          strTemp += Convert.ToDateTime(((DataRowView)dgrRow.DataBoundItem)["Hst_trandt"]).ToString("MM/dd/yyyy");
          strTemp += "  ";
          strTemp += ((DataRowView)dgrRow.DataBoundItem)["Hst_DrAmount"].ToString().PadLeft(15, ' ');
          strTemp += "  ";
          strTemp += ((DataRowView)dgrRow.DataBoundItem)["Hst_CrAmount"].ToString().PadLeft(15, ' ');
          strTemp += "  ";
          strTemp += ((DataRowView)dgrRow.DataBoundItem)["Hst_trancd"].ToString().PadLeft(4, ' ');
          strTemp += "  ";
          strTemp += GetDescriptionString(((DataRowView)dgrRow.DataBoundItem)["Hst_desc"].ToString(), 30, 62);
          strBuild.AppendLine(strTemp);
      }
      strCreateGridString = strBuild.ToString();
      return strCreateGridString;//strBuild.ToString();
  }
  

• Pas de clés primaires, des index ou des contraintes de clés étrangères existait sur les tables, presque tous les champs de type varchar(50), et 100% des champs ont été nullable. Fait intéressant, les champs de bits ne sont pas utilisés pour stocker des données booléennes; au lieu d'un char(1) le terrain a été utilisé, et les caractères 'Y' et 'N' utilisé pour représenter le vrai et le faux, respectivement.

  • En parlant de la base de données, voici un exemple représentatif d'une procédure stockée:

    ALTER PROCEDURE [dbo].[Get_TransHist]
     ( 
          @TellerID   int = null,
          @CashDrawer int = null,
          @AcctNum    bigint = null,
          @StartDate  datetime = null,
          @EndDate    datetime = null,
          @StartTranAmt     decimal(18,2) = null,
          @EndTranAmt decimal(18,2) = null,
          @TranCode   int = null,
          @TranType   int = null
     )
    AS 
          declare @WhereCond Varchar(1000)
          declare @strQuery Varchar(2000)
          Set @WhereCond = ' '
          Set @strQuery = ' '
          If not @TellerID is null
                Set @WhereCond = @WhereCond + ' AND TT.TellerID = ' + Cast(@TellerID as varchar)
          If not @CashDrawer is null
                Set @WhereCond = @WhereCond + ' AND TT.CDId = ' + Cast(@CashDrawer as varchar)
          If not @AcctNum is null
                Set @WhereCond = @WhereCond + ' AND TT.AcctNbr = ' + Cast(@AcctNum as varchar)
          If not @StartDate is null
                Set @WhereCond = @WhereCond + ' AND Convert(varchar,TT.PostDate,121) >= ''' + Convert(varchar,@StartDate,121) + ''''
          If not @EndDate is null
                Set @WhereCond = @WhereCond + ' AND Convert(varchar,TT.PostDate,121) <= ''' + Convert(varchar,@EndDate,121) + ''''
          If not @TranCode is null
                Set @WhereCond = @WhereCond + ' AND TT.TranCode = ' + Cast(@TranCode as varchar)
          If not @EndTranAmt is null
                Set @WhereCond = @WhereCond + ' AND TT.TranAmt <= ' + Cast(@EndTranAmt as varchar)
          If not @StartTranAmt is null
                Set @WhereCond = @WhereCond + ' AND TT.TranAmt >= ' + Cast(@StartTranAmt  as varchar)
          If not (@TranType is null or @TranType = -1)
                Set @WhereCond = @WhereCond + ' AND TT.DocType = ' + Cast(@TranType as varchar)
          --Get the Teller Transaction Records according to the filters
          Set @strQuery = 'SELECT 
                TT.TranAmt as [Transaction Amount], 
                TT.TranCode as [Transaction Code],
                RTrim(LTrim(TT.TranDesc)) as [Transaction Description],
                TT.AcctNbr as [Account Number],
                TT.TranID as [Transaction Number],
                Convert(varchar,TT.ActivityDateTime,101) as [Activity Date],
                Convert(varchar,TT.EffDate,101) as [Effective Date],
                Convert(varchar,TT.PostDate,101) as [Post Date],
                Convert(varchar,TT.ActivityDateTime,108) as [Time],
                TT.BatchID,
                TT.ItemID,
                isnull(TT.DocumentID, 0) as DocumentID,
                TT.TellerName,
                TT.CDId,
                TT.ChkNbr,
                RTrim(LTrim(DT.DocTypeDescr)) as DocTypeDescr,
                (CASE WHEN TT.TranMode = ''F'' THEN ''Offline'' ELSE ''Online'' END) TranMode,
                DispensedYN
          FROM TellerTrans TT WITH (NOLOCK)
          LEFT OUTER JOIN DocumentTypes DT WITH (NOLOCK) on DocType = DocumentType
          WHERE IsNull(TT.DeletedYN, 0) = 0 ' + @WhereCond + ' Order By BatchId, TranID, ItemID'    
          Exec (@strQuery)
    

Avec tout ce que dit, le seul gros problème avec cette ligne de 130 000 mise en application le présent: pas de tests unitaires.

Oui, j'ai envoyé cette histoire de TheDailyWTF, et puis j'ai quitté mon emploi.

Answer 2

J’ai vu une fonction de chiffrement de mot de passe comme ceci

Answer 3

Dans un système qui a eu des paiements par carte de crédit nous permettant de mémoriser le numéro de carte de crédit complet avec nom, date d’expiration etc.

Il s’avère que c’est illégal, ce qui est ironique étant donné la nous devais écrire le programme pour le ministère de la Justice à l’époque.

Answer 4

Ce fut l’erreur de manipulation de routine dans un morceau de code de commerce :

Je devais pour savoir pourquoi « le soft conserve enfermer ».

Answer 5

La combinaison de tous les éléments suivants Php "Caractéristiques" à la fois.

1. Register Globals
2. Les Variables
3. L'Inclusion de fichiers distants et le code via include("http:// ... ");

4. Vraiment Horrible Tableau/les noms de Variables ( Littérale exemple ):

   foreach( $variablesarry as $variablearry ){
     include( $$variablearry ); 
   }
   

   ( J'ai littéralement passé une heure à essayer de comprendre comment cela a fonctionné avant, j'ai réalisé qu'ils wern pas la même variable )

5. Inclure 50 fichiers, qui comprennent chacune 50 fichiers, et des choses est effectuée de façon linéaire/de la procédure dans tous les 50 fichiers dans conditionnelle et de façon imprévisible.

Pour ceux qui ne connaissent pas les variables:

$x = "hello"; 
$$x = "world"; 
print $hello # "world" ;

Considérons maintenant $x contient une valeur à partir de votre URL ( register globals la magie ), de sorte que nulle part dans votre code est-il évident que ce qui variable votre travail avec parce que ses déterminée par l'url.

Maintenant, considérons ce qui se passe lorsque le contenu de cette variable peut être une url spécifiée par les sites web de l'utilisateur. Oui, cela peut ne pas faire sens pour vous, mais il se crée une variable nommée cette url, c'est à dire:

$,

sauf qu'il ne peut pas être directement accessibles, vous avez de l'utiliser via la double $ technique ci-dessus.

En outre, lorsque son possible pour l'utilisateur de spécifier une variable dans l'URL qui indique le fichier à inclure, il y a des mauvais tours comme

http://google.com

et si cette variable tourne en

et 'evilcode.php' imprime son code en clair, et Php est mal sécurisé, php va gambader tout off, télécharger evilcode.php et de l'exécuter en tant qu'utilisateur du serveur web.

Le serveur web va lui donner toutes ses autorisations, etc, permettant shell appels, le téléchargement des fichiers binaires arbitrairement et en cours d'exécution, etc etc, jusqu'à ce que finalement vous vous demandez pourquoi vous avez une boîte de l'espace disque disponible, et un dir a 8 GO de films piratés avec l'italien doublage, partagé sur IRC via un bot.

Je suis juste heureux que j'ai découvert cette atrocité avant l'exécution du script à l'attaque décidé de faire quelque chose de vraiment dangereux, comme la récolte des informations extrêmement confidentielles à partir de la plus ou moins non garantie de base de données :|

( J'ai pu connaître de la dailywtf tous les jours pendant 6 mois avec ce code, je ne plaisante pas. C'est juste une honte, j'ai découvert le dailywtf après j'ai échappé à ce code )