五种能蒙混代码审查的欺骗性 Node.js 漏洞

通过五个涉及 forEach、浮动承诺和浅拷贝的实际 Node.js 错误案例，了解为何原本运行正常的代码在生产环境中仍可能出错。

1485 词

下面的五个代码示例均能正常执行而不会抛出异常，很可能在快速的代码审查中就被忽视。然而它们每一个都曾在某些生产系统中多次引发真正的服务中断。在阅读每段代码下方的解释之前，先试着自己找出问题所在。

错误 1

async function notifyAllUsers(userIds) {
  userIds.forEach(async (id) => {
    const user = await getUser(id);
    await sendNotification(user);
  });
  console.log("All notifications sent!");
}

暂停此处，思考这段代码运行时实际发生了什么。

错误所在：在任何通知真正发送之前，console.log("所有通知已发送！") 这行代码就已经执行了，而且外部函数本身也在未等待各个发送操作完成的情况下就结束了执行。

原因是 Array.prototype.forEach 没有处理承诺的概念。它会为每个元素调用回调函数，然后立即转向下一个元素，完全忽略回调函数返回的值。将回调函数声明为 async 也不会改变 forEach 的行为方式——这只是意味着每次调用都会生成一个承诺，而 forEach 会直接丢弃这些承诺而不进行任何检查。通知最终仍会被发送，只是以异步方式在后台执行，没有固定的顺序，调用方也无法检测到任务的完成或失败。

async function notifyAllUsers(userIds) {
  await Promise.all(userIds.map(async (id) => {
    const user = await getUser(id);
    await sendNotification(user);
  }));
  console.log("All notifications sent!");
}

改用 map 后，这些承诺会被收集到数组中而非被丢弃，再将该数组用 Promise.all 包装，这样就能确保函数真正等待所有操作完成。现在的日志信息显示的是真实情况。

错误 2

app.post("/orders", async (req, res) => {
  const order = await createOrder(req.body);
  sendConfirmationEmail(order.customerEmail);
  res.status(201).json(order);
});

这条路由似乎运行正常——订单被创建，邮件被发送，响应也很快返回。那问题出在哪里呢？

问题所在：sendConfirmationEmail函数被调用时未使用await，因此其返回的承诺对象完全处于无人管理的状态。若该承诺被拒绝，将没有任何机制来捕获这一异常。这种模式被称为“悬空承诺”，它不仅仅是个风格问题，更是一种运行风险。在当前版本的Node.js中，未被处理的承诺拒绝可能会导致整个进程崩溃，使其他正在处理的请求一同失败，而不会只是静默地发送邮件失败。

除了缺失的错误处理之外，这里还隐藏着一个重要的架构问题：失败的确认邮件是否应该阻止订单完成，还是无论如何都应让订单继续处理？在大多数情况下，人们希望选择后者——即使没有收到通知，订单也应视为已成功处理。但选择不因某件事而阻止流程与完全无法处理其错误之间是有区别的，而这段代码很可能本意是前者，却意外地做到了后者。

app.post("/orders", async (req, res) => {
  const order = await createOrder(req.body);

 sendConfirmationEmail(order.customerEmail).catch((err) => {
    logger.error({ orderId: order.id, err }, "Failed to send confirmation email");
  });

  res.status(201).json(order);
});

在这个版本中，邮件确实不会阻碍HTTP响应的发送，但错误现在会被记录下来，而不会悄无声息地消失或导致服务器崩溃。

错误3

function applyDiscount(cart) {
  const updatedCart = { ...cart };
  updatedCart.items.forEach((item) => {
    item.price = item.price * 0.9;
  });
  return updatedCart;
}

乍看之下，这似乎符合那些关于避免副作用的指南中提到的标准“复制而非修改”原则。但实际上它是一个更为隐蔽的陷阱。

问题所在：展开运算符 { ...cart } 只会进行浅拷贝。它虽然在最顶层创建了一个新对象，但 updatedCart.items 仍然指向与 cart.items 完全相同的数组以及相同的元素对象。因此，当 forEach 循环修改 item.price 时，实际上也在无形中修改了原始购物车中的商品信息，因为展开运算符从未触及结构的第一层之外的任何内容。

console.log(cart.items[0].price);        // already discounted, unintentionally
console.log(updatedCart.items[0].price); // same object, same value

那些认为原始 cart 不会被修改的人——鉴于函数名称暗示它会返回新数据，这种期望其实合情合理——最终处理的却是已被悄悄破坏的数据。

function applyDiscount(cart) {
  return {
    ...cart,
    items: cart.items.map((item) => ({ ...item, price: item.price * 0.9 })),
  };
}

每个需要修改的嵌套层都必须在该层进行显式复制。浅层展开仅能保护其直接操作的层级，无法保护其下方的嵌套内容。

错误 4

function updateUserSettings(user, updates) {
  return Object.assign({}, user, updates);
}

app.patch("/settings", (req, res) => {
  const updated = updateUserSettings(req.user, req.body);
  saveUser(updated);
  res.json(updated);
});

这看起来像是一个普通的“将更新对象合并到现有对象中”的功能。那么危险究竟隐藏在哪里呢？

错误所在： req.body 直接来自客户端，此处没有任何机制限制哪些键可以被纳入用户对象。如果请求体中包含类似 "role": "admin" 或 "isVerified": true 的内容，这些属性会像任何合法的设置字段一样被轻易合并进去，因为 Object.assign 并不区分哪些键应该是可写的——它会合并所有传入的属性。

这属于一种名为“批量赋值”的常见漏洞类型，经常出现在那些不先通过允许列表过滤就直接将请求体插入数据库模型的API中。随着合并函数变得更加通用和可复用，风险实际上会进一步增加——而这恰恰是这类代码最初让人觉得可靠的原因。

function updateUserSettings(user, updates) {
  const allowedFields = ["displayName", "timezone", "emailNotifications"];
  const safeUpdates = {};
  for (const field of allowedFields) {
    if (field in updates) safeUpdates[field] = updates[field];
  }
  return { ...user, ...safeUpdates };
}

通过明确定义允许列表，无论有人在请求载荷中添加了哪些额外的键值，都能确保请求绝不会影响到那些未被明确允许修改的字段。

错误5

async function getUserWithPosts(userId) {
  const user = await db.query("SELECT * FROM users WHERE id = $1", [userId]);
  const posts = await db.query("SELECT * FROM posts WHERE user_id = $1", [userId]);
  return { ...user, posts };
}

从技术层面来看，这段代码并没有任何问题。但以这种方式编写代码会让你付出什么代价呢？

问题所在——或者更确切地说，是错失的机会——在于这两个查询彼此之间完全没有依赖关系。第二个查询在运行之前根本不需要第一个查询的任何结果。通过使用连续的await调用将它们串联起来，总等待时间就会变成两个查询耗时之和，而且其中一个还会毫无必要地阻塞另一个。

async function getUserWithPosts(userId) {
  const [user, posts] = await Promise.all([
    db.query("SELECT * FROM users WHERE id = $1", [userId]),
    db.query("SELECT * FROM posts WHERE user_id = $1", [userId]),
  ]);
  return { ...user, posts };
}

使用 Promise.all 可以让两个查询同时执行，而非依次执行，因此总耗时大致取决于较慢的那个查询，而不是两个查询耗时的总和。这并非错误，因为它并不会产生错误结果——顺序执行版本也能返回完全准确的数据。问题在于它悄悄浪费了可用的性能，而且这种写法往往是出于习惯而形成的，很少有人会质疑，因为乍看之下代码并无异常。

这五种方法的共同点

这些代码片段均能无错误运行，输出结果也看似合理，稍加查看便能通过检验。但若仅以“试运行一次是否正常”作为测试标准，是根本发现不了问题的。要找出这些问题，需要具备一种特定的怀疑态度：这段异步代码真的在等待它所声称要等待的所有内容吗？这个复制操作确实复制了所有必需的层吗？这种合并逻辑真的会信任那些本不应信任的输入数据吗？这样的直觉并非来自对更多语法的记忆，而是源于曾经至少一次遭遇过这五种特定模式带来的麻烦。