net 模块,参数传错就进程崩溃

Posted by Lxxyx on 2021-01-17

近期给 Node.js 提了许多 PR。这当中有一些 BUG 我觉得还挺有意思的,所以开个专栏专门讲述 BUG 的定位、修复、提 PR 的过程。

话不多说,先来看第一个 BUG:使用 net 模块的 BlockList 类时,如果调用 addSubnet 的第 2 个参数为 NaN,进程就会 Crash 退出。

Issue 由我发现和修复,目前已合并至 Node.js 主分支中,并在 15.6.0 版本中分发。

Github 上提交的 Issue 和 PR:

复现

Node 版本: 15.4.0

问题代码如下:

1
2
3
const net = require('net');
const blockList = new net.BlockList();
blockList.addSubnet('', NaN);

报错信息:

1
2
3
4
5
6
> blockList.addSubnet('', NaN)
node[2155]: ../src/node_sockaddr.cc:614:static void node::SocketAddressBlockListWrap::AddSubnet(const FunctionCallbackInfo<v8::Value> &): Assertion `args[2]->IsInt32()' failed.
1: 0x101379d05 node::Abort() (.cold.1) [/usr/local/bin/node]
2: 0x1000bde89 node::Abort() [/usr/local/bin/node]
....
[1] 2155 abort node

而这种在 C++ 抛出的异常,是你用 Try/Catch 也捕获不了的,如果你传入了错误的参数,那么进程只能 Crash。

定位

net 模块中,BlockList 负责网络黑名单的工作,用于屏蔽某些 IP 地址。本次出问题就是其中的 addSubnet 方法。

在 Node.js 的源码中,所有 JS 提供的功能都在 lib 目录下,简单的搜索 BlockList 关键词,我定位到了本次发生问题的代码文件及方法:https://github.com/nodejs/node/blob/37a8179673590af10b9e8e413388adffc21ba713/lib/internal/blocklist.js#L81-L105
image.png

addSubnet 方法中,会对传入 prefix 参数做校验,而这个校验实际上是存在漏洞的。

在使用 typeof 来校验 number 时,往往会遗漏 NaN 的校验,因为 typeof NaN === 'number' 。此次的问题也是如此。

在最终调用 this[kHandle].addSubnet(network, type, prefix); 时我们传入了 NaN,而 this[kHandle] 是 C++ 的 binding。

1
2
3
4
5
6
7
8
9
const {
BlockList: BlockListHandle,
} = internalBinding('block_list');

class BlockList {
constructor(handle = new BlockListHandle()) {
this[kHandle] = handle;
}
}

在 C++ 代码中,则对传入的参数又做了一次校验。

image.png

NaN 不是 Int32,因此将导致进程异常退出(exit code: 134,代表中断)。

修复

了解问题后,修复并不困难,只要在 JavaScript 侧加入对 NaN 的校验即可。

而在 Node.js 的内部提供了 validateInt32 方法,用于校验数字。我们只需要将原来的 if (typeof prefix !== 'number') 替换为 validateInt32 即可。

下面是完整的 Diff。
image.png

单测

在 Node.js 源码中,任何变更都需要配上相应的单元测试,此次也不例外。

我在 test/parallel/test-blocklist.js 加入了单元测试,本地测试通过后便提交了 Pull Request。

单元测试的代码:

1
assert.throws(() => blockList.addSubnet('', NaN), /ERR_OUT_OF_RANGE/);

提交 & 合入主分支

相关的 Pull Request: net: throw ERR_OUT_OF_RANGE if blockList.addSubnet prefix is NaN

因为改动不大,且 BUG 确实存在,因此这个 PR 没有争议的迅速通过了。
image.png
提出 PR 大概一周的时间,代码就已经被合入 Node.js 主分支,并在 Node.js 15.6.0 中分发到全世界了(顺带一提,在 15.6.0 版本中,我有 12 个 commit 被合入当前版本)。
image.png
image.png

后续

全文看下来,你会发现往 Node.js 提交 Pull Request 并不困难,且修复 BUG 的过程也是熟悉源码的过程。感兴趣的话不妨阅读 Node.js 的文档:《Contributing to Node.js》 与 Starkwang 的文章:《为 Node.js 贡献你的力量》

后续我也会持续更新这个专栏,将我解决过的 Issue 写成文字,供大家学习和参考,有兴趣的同学可以点个关注~