解析世界上最糟糕的代码

有一个意大利Facebook页面。它被称为“ Il Programmatore di Merda”,意为“糟糕的程序员”。我喜欢这个页面。



他们经常发布有关编程的令人反感的代码和模因。但是有一天我在那里看到了绝对令人惊奇的东西。





这段代码已获得本周“最佳作品”的荣誉称号。



我决定反汇编这段代码,但是有太多错误的事情,即使选择第一个问题进行分析,我也很难。



如果您是一名初学者程序员,那么我的材料将帮助您了解编写此代码的人犯了哪些严重的错误。



28行代码错误



为了更方便,让我们在编辑器中键入以下代码:



<script>
function authenticateUser(username, password) {
  var accounts = apiService.sql(
    "SELECT * FROM users"
  );

  for (var i = 0; i < accounts.length; i++) {
    var account = accounts [i];
    if (account.username === username &&
        account.password === password)
    {
      return true;
    }
  }
  if ("true" === "true") {
    return false;
  }
}

$('#login').click(function() {
  var username = $("#username").val();
  var password = $("#password").val();

  var authenticated = authenticateUser(username, password);

  if (authenticated === true) {
    $.cookie('loggedin', 'yes', { expires: 1 });
  } else if (authenticated === false) {
    $("error_message").show(LogInFailed);
  }
});
</script>


而且,嗯,真的-我不知道从哪里开始分析它。但是您仍然需要开始。我决定将这段代码的缺点分为三类:



  • 安全问题。
  • 基本的编程知识问题。
  • 代码格式问题。


安全问题



此代码可能在客户端上运行。事实是它被包装在一个标签中<script>(此外,这里使用了jQuery)。



但是不要误会我的意思。如果用于服务器,则此代码看起来同样糟糕。但是在客户端上执行它意味着能够读取此代码的每个人都可以访问其中使用的数据库。



让我们看一下函数authenticateUser



function authenticateUser(username, password) {
  var accounts = apiService.sql(
    "SELECT * FROM users"
  );

  for (var i = 0; i < accounts.length; i++) {
    var account = accounts [i];
    if (account.username === username &&
        account.password === password)
    {
      return true;
    }
  }
  if ("true" === "true") {
    return false;
  }
}


通过对其代码的分析,我们可以得出结论,某个地方存在一个对象apiService该对象为我们提供了一种方法,.sql通过该方法可以对数据库执行SQL查询。这意味着,如果您在查看包含此代码的页面时打开开发人员控制台,则可以对数据库执行任何查询。



例如,您可以运行如下查询:



apiService.sql("show tables;");


作为响应,将返回数据库表的完整列表。所有这些都是使用项目自己的API完成的。但是,让我们想象一下这不是一个真正的问题。让我们更好地看一下:



if (account.username === username &&
    account.password === password)


此代码告诉我密码存储在项目中,没有哈希。大招!这意味着我可以使用调试器并查看项目用户的密码。我还相信,很大一部分用户在社交媒体,电子邮件服务,银行应用程序和其他类似地方使用相同的用户名/密码对。





我也看到了如何在此代码中设置cookie的问题loggedin



$.cookie('loggedin', 'yes', { expires: 1 });


事实证明,它使用jQuery设置一个cookie,该cookie告诉Web应用程序用户是否已通过身份验证。



请记住:切勿使用JavaScript设置这些cookie。



如果您需要在客户端上存储此类信息,则通常使用cookie。我不能对这种想法说不好。但是,使用JavaScript设置cookie意味着无法配置该属性httpOnly。反过来,这意味着任何恶意脚本都可以访问这些cookie。



是的,我知道这里只存储一个键:值对,'loggedin': 'yes',因此即使其他人的脚本读取了类似的内容,也不会造成太大的伤害。但是无论如何这都是非常不好的做法。



另外,当我打开Chrome控制台时,总是可以输入$.cookie('loggedin', 'yes', { expires: 1000000000000 });结果,即使没有帐户,我也已经永远进入了该网站。



基本的编程知识问题



我们可以讨论并讨论这段代码中可能遇到的类似问题,而且我们没有太多时间。



因此,显然,函数authenticateUser是编写非常糟糕的代码的一个示例。它表明编写它的人缺乏一些编程方面的基本知识。



function authenticateUser(username, password) {
  var accounts = apiService.sql(
    "SELECT * FROM users"
  );

  for (var i = 0; i < accounts.length; i++) {
    var account = accounts [i];
    if (account.username === username &&
        account.password === password)
    {
      return true;
    }
  }
  if ("true" === "true") {
    return false;
  }
}


也许不是从数据库中获取完整的用户列表,而是选择具有给定名称和密码的用户就足够了吗?如果数以百万计的用户存储在该数据库中会怎样?



我已经讨论过了,但是我将重复我自己,问一个问题:“为什么他们不对数据库中的密码进行哈希处理?”



现在让我们看一下该函数返回什么authenticateUser。从我所看到的,我可以推断出它接受两个类型参数string并返回一个类型值boolean



因此,下面的代码虽然写得很糟,但不能称其为无意义:



for (var i = 0; i < accounts.length; i++) {
  var account = accounts [i];
  if (account.username === username &&
      account.password === password)
  {
    return true;
  }
}


如果将其翻译成普通语言,则会得到以下信息:“是否有一个名为X且密码为Y的用户?如果是,则返回true。



现在让我们看一下这段代码:



if ("true" === "true") {
  return false;
}


废话。为什么函数不返回false为什么她需要一个始终为真的条件?



现在让我们分析以下代码:



$('#login').click(function() {
  var username = $("#username").val();
  var password = $("#password").val();

  var authenticated = authenticateUser(username, password);

  if (authenticated === true) {
    $.cookie('loggedin', 'yes', { expires: 1 });
  } else if (authenticated === false) {
    $("error_message").show(LogInFailed);
  }
});


此代码的jQuery部分看起来不错。但是这里的问题是使用cookie进行工作的组织loggedin



我们已经说过,即使没有帐户,您也可以简单地打开Chrome控制台并运行命令$.cookie('loggedin', 'yes', { expires: 1 });并进行一天的身份验证。



该页面如何知道经过身份验证的用户是谁?也许它只是显示一些特殊的内容,仅适用于那些成功通过用户名和密码验证并且不显示任何个人数据的用户?我们不会知道这一点。



代码格式问题



样式可能是此代码中最小,最无关紧要的问题。但是它清楚地表明创建此代码的人从某个地方复制并粘贴了其各个片段。



这是格式化字符串时使用双引号的示例:



var username = $("#username").val();
var password = $("#password").val();


在其他地方,使用单引号:



$.cookie('loggedin', 'yes', { expires: 1 });


听起来似乎不重要,但实际上告诉我们,开发人员可能已经按照项目中使用的样式指南从StackOverflow复制了一些代码,甚至没有重写它们。当然,这是一个很小的细节,但是它表明开发人员并不真正在乎理解代码的工作原理。该开发人员只需要代码以某种方式起作用即可。



我想在此阐明我对这个问题的观点。我每天都在Google上搜索与代码相关的内容,但我认为,例如,如何设置Cookie而不是使代码从某处工作中被无意地复制,这一点非常重要。如果程序由于某种原因停止工作怎么办?不了解代码中正在发生什么的程序员如何发现错误?



结果



我绝对确定这段代码是伪造的。在这里,我首先看到一个同步执行SQL查询的示例:



var accounts = apiService.sql(
  "SELECT * FROM users"
);


通常,这些任务可以按以下方式解决:



var accounts = apiService.sql("SELECT * FROM users", (err, res) => {
  console.log(err); // 
  console.log(res); //   
});


他们也这样解决:



var accounts = await apiService.sql(
  "SELECT * FROM users"
);


即使该方法apiService.sql以同步模式返回结果(我对此表示怀疑),它也需要打开与数据库的连接,执行请求,然后将结果发送给调用点。并且这些操作(您可能会猜到)不能同步执行。



但是,即使这是完全真实的代码,也可以肯定它是由初级程序员初级编写的。我敢肯定,当我成为程序员的最初几周中,我还编写了糟糕的代码(对不起:D)。



但这不是初级程序员的错。



假设这是在某个地方运行的真实代码。一些初级人员会全力以赴使此代码正常工作。该开发人员仍然必须学习如何正确处理SQL查询,Cookie以及其他所有内容。因此,这是完全正常的。



但是,该程序员的负责人必须控制工作,指出错误并让初学者了解他的错误。这将导致这样一个可怕的代码无法进入生产的事实。



我肯定知道有些公司不关心投入生产的代码的质量。代码是否可以解决问题?如果是这样,则可以毫无问题地部署它。代码是由初级编写的,甚至没有经过经验丰富的程序员的测试吗?在生产中!



总的来说,生活中有悲伤。



截至2020年8月8日更新



在撰写本文时,我认为我正在分析的代码是伪造的。但是在讨论有关Reddit的资料之后我得到了与线程的链接事实证明,此代码在支持1,500个用户的某些内部系统中使用。所以我显然是错的。这是完全真实的代码。



您是否在生产中遇到过坦率的错误代码片段,充满了各种各样的问题?






All Articles