安全人员的代码水平

代码分析

滥用 globals

这段代码中，很多 globals 的使用都是不必要的，这种会破坏代码的逻辑结构，而且可能会带来潜在的并发问题。正确的办法应该是在调用方接受函数返回值，然后继续传递给下一个函数。

可以使用原生代码的逻辑结果是调用命令

本漏洞是一个目录穿越，只要发送 ../ 这种的请求而且穿越成功即可，本来使用 Python urllib 两行的事情，这个人却使用了 curl 执行命令来检查返回值实现的，这种用法主要有以下缺点

1. 新启动进程，耗费资源，降低代码性能
2. 潜在的命令注入问题，更何况这里使用了 shell=True 参数。这个可以借助下面的代码来理解

>>> subprocess.check_output("curl http://example.com; expr 1024 + 20480000", shell=False)
Traceback (most recent call last):
  ......
FileNotFoundError: [Errno 2] No such file or directory: 'curl http://example.com; expr 1024 + 20480000': 'curl http://example.com; expr 1024 + 20480000'
>>> subprocess.check_output("curl http://example.com; expr 1024 + 20480000", shell=True)
b'\n\n\n    .....\n20481024\n'

一个重大逻辑错误

如果说上面的问题不影响 poc 的效果的话，下面这个问题是真正的错误了。

if ("[global]") and ("encrypt passwords") and ("name resolve order") in str(response):    pass

根据漏洞原理和代码写法猜测，原作者的意思是 response 中同时含有这三个字符串，但是这里的写法却错误的理解了 Python 的优先级。
这个代码等价于

True and True and ("name resolve order") in "name resolve order"

只要 response 含有最后一个字符串就会是 True，实际应该为

"str1" in response and "str2" in response

这种写法。

编码解码的问题

还是在这段代码

if ("[global]") and ("encrypt passwords") and ("name resolve order") in str(response):    passelif NSIP_RESPONSE_MSG in str(response.decode("utf-8", errors="ignore")):    pass

很多处没必要 bytes 转 string，直接去 bytes 匹配即可，毕竟匹配的都是英文单词，不涉及到解码的问题。
代码在转换 curl 输出为 string 的时候，上面就看到了两种写法。而查看 Python 的文档，str 的实现是 class str(object=b”, encoding=’utf-8′, errors=’strict’)，也就是说第二处 errors=”ignore” 根本没有用处，如果真的发生错误，在 str 处就异常了。

其他的问题

1. 没有必要去 check_valid_host 捕获 Python 或者 curl 的异常即可，现在的写法需要去解析域名结果，又浪费了性能
2. 读取文件直接使用了 readlines，而不是 readline，这样会直接读取整个文件到内存，大文件的时候可能会 oom。
3. 结果是最后写入文件的，如果中间代码发生异常，所有的结果就都丢了，如果检测一个写一个，会好一些。
4. 没有使用多线程，ip 数量多的时候速度可能会比较慢，但是我估计他们如果使用了多线程，肯定就会有全局变量的竞争问题。

看完代码之后，感觉就是这是一个安全人员写的代码，因为自己去检测这个漏洞只需要一个 curl 就够了，而讲这个命令转换为 Python 检测代码，这人的想法就是修修补补，比如发请求就去 subprocess 之前的 curl 就可以了，比如 shell 变量中使用 pipe 或者文件来存储中间结果，现在就直接全局变量。

如何和社区安全人员合作开发

xray 是长亭科技的洞鉴扫描器引擎中的一部分，社区版目前可以免费下载使用。它在2019年进行了一个彻底的重构，这主要分为两部分，引擎主体和 poc 由社区主导。
xray 和社区合作的主要原因是

1. 团队人手不够，无法短时间完成大量 poc，之前即使有安服师傅的帮助，也数量有限
2. 开源 poc 写法五花八门，也没有一个好的归类和运行平台，无法直接使用
3. xray 由 Golang 编写而且不开源，不像 Python 或者 Ruby 等实现动态的代码执行

和社区合作肯定会遇到上一节提到的问题，社区人员代码水平参差不齐，如何最大化保证规范和质量就是最重要的问题了。我们最终的决定是这样的

1. 不提供任意代码的执行能力
2. 静态类型，尽可能少的 RuntimeError
3. 尽可能提供更多的静态检查工具

以下简单的分析下这三点

不提供任意任意代码执行能力

任意代码执行能力的实现一般是嵌入其他语言解释器来实现的，比如 jaeles-project/jaeles 是参考了 xray poc 架构的一个扫描器，它的 poc 模块和 xray 的写法很像，比如 StatusCode() == 200 && !StringSearch(“response”, “Not Found”)，它的 runtime 的实现就是 JavaScript。
长亭科技实践中在 Golang 中嵌入 Lua 解释器的经验最为丰富，因为我们的场景都要求高性能，我们基于开源的实现进行了一些修改和优化，借助 Lua 的轻量虚拟机和 jit 不难做到这一点。使用场景有在 WAF 上使用 Lua 编写流量检查逻辑和执行流量分析 SQL 等，主机安全 agent 上使用 Lua 编写自定义插件，实现基线检查、数据监控等。
提供任意代码执行能力就代表 poc 编写者发挥的余地最大，同一个逻辑可以有很多写法，并不太适合能力参差不齐的多人合作。
比如扫描器都会提供一些 http 参数的配置，这些只有使用自带的 http client 才可以生效，以 Python 为例可以是 self.client.get(url, **kwargs)，如果放开了任意代码执行的能力，你可能就会看到 requests.get、urllib.request.urlopen 等无数种写法。比如一个 HTTPException 应该是 poc 代码中捕获还是不捕获传递到上层，这些即使有文档的规范也无法强制代码编写者遵守。这些问题在一些开源的扫描器引擎中也经常看到，这也是安全开发人员需要考虑的基础架构问题。
而且任意代码执行能力存在潜在的安全问题，至少需要屏蔽掉一些危险的库。
我们仔细想了下，在实际的 poc 场景下，最常见的就是发送一个或者多个请求，然后匹配和提取 response 的数据，这样的流程很容易使用 yaml 这种格式化数据来表示。以一个 sql 注入的 poc 为例

name: poc-yaml-nagio-cve-2018-10738
set:
  r: randomInt(2000000000, 2100000000)
rules:
  - method: POST
    path: /nagiosql/admin/menuaccess.php
    headers:
      Content-Type: application/x-www-form-urlencoded
    body:
      selSubMenu=1&subSave=1&chbKey1=-1%' and (select 1 from(select count(*),concat((select (select (select md5({{r}}))) from information_schema.tables limit 0,1),floor(rand(0)*2))x from information_schema.tables group by x)a)#
    follow_redirects: false

这样我们将使用代码发送 HTTP 请求的逻辑就转换为了这样的 yaml 数据，因为字段名是规定好的，所以自由发挥的余地并不大。

静态类型，尽可能少的 RuntimeError

Lua 语言的问题是语法过于简单，而且标准库和第三方库都不多，它和 JavaScript 共同的问题是动态语言，很多错误都是运行到那一行才会发现，甚至很多错误都不能发现。xray 在重构之前是使用 Python 编写的，即使有丰富经验的安全研发工程师也经常被坑。比如因为没有类型，想调用一个 api 得去看代码实现，看看返回值是什么类型，这个在 type hint 不全面的情况下非常拖累开发效率。甚至你可以看到 sqlmap 代码中很多函数不同情况下会有不同的返回值类型。
为什么 Golang 更适合做新版 xray 的开发？这个优点简直太多了，比如语法规范、强类型检查、大部分错误在编译阶段已经解决、效率高、并发编程简单、部署分发简单等等。如果可以让社区的 poc 也使用 Golang 编写就好了，但是事与愿违，主要原因就是 Golang 基本无法实现动态代码执行，而 Go Plugin 等技术目前也是残废和半死不活。
但是我们可以造轮子啊，实现一个和 Golang 语法类似的严肃的语言和 runtime，后续我们通过调研选择了 cel 表达式，这个是 Google 的开源项目，代码和实现都非常优美和高效。cel 表达式的函数和参数都有固定的类型，必须显式的指定，否则就会报错。参见上面的 poc，我们匹配漏洞存在的表达式就可以是 response.body.bcontains(bytes(md5(string(r))))，因为response.body 是 bytes 类型的，如果你没有使用 bytes 转换一个 string，那在 xray 的 poc 检查或者初始化时期就可以发现。

ERROR: :1:24: found no matching overload for 'bcontains' applied to 'bytes.(string)'
 | response.body.bcontains(md5(string(r)))
 | .......................^

除此之外，我们来提供了一系列的静态类型安全的函数，比如正则表达式函数、哈希函数、字符串处理函数等等。

尽可能提供更多的静态检查工具

我们的 poc 框架，目的之一让不太懂代码的安全人员也能方便上手来写 poc，而且尽可能的帮助他们写出规范优美的 poc。即使有了上面的那些限制，一个 poc 还是有很多灵活性，比如 response.body == b”test” 还是 response.body == b’test’ 还是 response.body==b”test”，这三种在语义上是一样的。
Golang 还有一个杀手锏就是自带 go fmt，可以将全世界所有的 Golang 代码格式化为同一个风格，这样开发人员再也不需要争论代码风格到底要怎么写。有了这个启发，xray 也提供了 poclint 工具，可以帮助你检查出 poc 中的不规范之处。下面就是一个样例的检查结果

Checking rule filename
filename 33.yml and poc name  do not match, maybe it should be poc-yaml-33Checking rule yamlschemaFile: /tmp/33.ymlI[#] S[#/required] missing properties: "name"Checking rule yamllint
running  /usr/local/bin/yamllint -c /var/folders/yr/bbl5fz_s5bd8mtfyw5rbgxhm0000gn/T/196546227 -f colored /tmp/33.yml
/tmp/33.yml  1:2       error    missing starting space in comment  (comments)  6:1       error    too many blank lines (3 > 1)  (empty-lines)
Checking rule cellintFile: /tmp/33.ymlCurrent :  response.body.bcontains(md5(string(r))) && response.status_code== 200Expected:  response.body.bcontains(md5(string(r))) && response.status_code == 200

检查出来的问题就包括 yaml 文件名不对、poc 中缺少 name 字段、yaml 中多了空行和 expression 表达式少了空格等等。
我们在 Github 上收录 poc 之前也会自动化的去检查你的 poc，如果有格式问题需要先改正。

安全研发

如果一个人安全也懂一些，研发也懂一些，那就是符合安全开发这个岗位了，这个岗位在各大公司中主要是做扫描器引擎、WAF/IPS引擎、风控类、网络测绘、内部安全体系建设等等。
最近面试过很多人，真正让人满意的安全研发是太太太稀缺了，很多安全比较厉害的人，研发就是上面的水平，很多研发还可以的人，是不怎么懂安全的。当然，我一直认为，一个研发大佬学习安全是没太大难度的，最主要的还是缺少安全大佬积累的奇技淫巧、对安全技术的热情等等，这些都阻碍着招聘的进度。一种解决方案是将安全研发拆分为安全研究和研发，安全研究团队负责研究技巧。给出思路和 demo 实现，然后由研发团队去进行产品化的实现。
*本文原创作者：virusdefender，本文属于FreeBuf原创奖励计划，未经许可禁止转载