如果 CVE-2022-25517 对于 mybatis plus 是合适的,那么 mybatis 本身也应该 own 一个类似的 CVE issue - V2EX
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
请不要在回答技术问题时复制粘贴 AI 生成的内容
BraveXaiver
V2EX    程序员

如果 CVE-2022-25517 对于 mybatis plus 是合适的,那么 mybatis 本身也应该 own 一个类似的 CVE issue

  •  
  •   BraveXaiver 2024-09-17 09:21:24 +08:00 3418 次点击
    这是一个创建于 391 天前的主题,其中的信息可能已经有所发展或是发生改变。

    最近公司引入了 SYNK 用作软件漏斗扫描工具,它给 mybatis plus 报 CVE-2022-25517 ,如果解决不了我就得把好大段代码用 JPA 或者 mybatis dynamic sql 重构。但我研究了下,我真心觉得这个如果是个有效的 CVE issue ,那 mybatis 也跑不了。

    以下是正文。各位看了后觉得呢?

    Background

    Here is the POC and root cause why CVE-2022-25517 was generated: POC

    But without mybatis-plus, only using mybatis, we can reproduce the same issue. Please check this demo: https://github.com/XSun771/demos/tree/mybatis-sql-injection

    By this demo, I want to prove that CVE-2022-25517 should not be an CVE issue. At least if it is, then it is also applicable to Mybatis. Instead, it should be a bad code smell.

    My POC

    code

    @Select("SELECT * FROM ARTICLES WHERE ${columnName} = #{columnValue}") List<Article> select(@Param("columnName") String columnName, @Param("columnValue") String columnValue); 

    I know you may say , "oh, it is highlighted by Mybatis developers that you should not use ${} but #{} which will check sql injection".

    But I must highlight that it is also highlighted in mybatis-plus official documents that everyone needs to do SQL inject check first.

    So if you agree that because mybatis developers highlights that you should not use ${} then there is no need to raise CVE issue for mybatis, then why not agree that no need to raise CVE issue to mybatis-plus?

    @RequestMapping("/enquiry") public String enquiry(@RequestBody Enquiry enquiry) { return this.articleMapper.select(enquiry.getColumnName(),enquiry.getColumnValue()).toString(); } 

    attack

    I made usage of IDEA http client, the script file is at src/main/resources/generated-requests.http.

    POST http://localhost:9000/enquiry Content-Type: application/json { "columnName": "(id=1) UNION SELECT * FROM ARTICLES WHERE 1=1 OR id", "columnValue": "1" } 

    Attachk result:

    2024-09-16T11:42:48.220+08:00 DEBUG 2736 --- [mybatis-sql-injection] [nio-9000-exec-2] c.e.m.ArticleMapper.select : ==> Preparing: SELECT * FROM ARTICLES WHERE (id=1) UNION SELECT * FROM ARTICLES WHERE 1=1 OR id = ? 2024-09-16T11:42:48.229+08:00 DEBUG 2736 --- [mybatis-sql-injection] [nio-9000-exec-2] c.e.m.ArticleMapper.select : ==> Parameters: 1(String) 2024-09-16T11:42:48.244+08:00 DEBUG 2736 --- [mybatis-sql-injection] [nio-9000-exec-2] c.e.m.ArticleMapper.select : <== Total: 3 [Article(id=1, title=foo, author=foo), Article(id=2, title=bar, author=bar), Article(id=3, title=333, author=333)] 
    22 条回复    2024-09-20 11:12:41 +08:00
    L0L
        1
    L0L  
       2024-09-17 09:35:47 +08:00
    建议是说尽量不要让接口传入$的参数值变量;如果不得不传入,一定要做好参数校验。这种很容易引发 sql 注入类的问题,
    leaflxh
        2
    leaflxh  
       2024-09-17 10:15:53 +08:00
    不知道是不是我代码写得少,通过让用户指明列名,来决定查询表的哪一个列,挺奇怪的功能
    BraveXaiver
        3
    BraveXaiver  
    OP
       2024-09-17 10:23:00 +08:00
    @leaflxh
    @L0L
    是的,我也同意这种写法不好。如果开发者这么写了,代码质量分析工具报问题,我没意见。但不应该视作对 mybatis-plus 这个库本身的 CVE issue 。如果视作,那么 mybatis 也应当适用此 CVE issue 。
    leohuangsulei
        4
    leohuangsulei  
       2024-09-17 11:10:20 +08:00
    一般不都是使用#{}吗?
    leohuangsulei
        5
    leohuangsulei  
       2024-09-17 11:11:37 +08:00
    一般啥需求会选择列名查询表。。
    wssy001
        6
    wssy001  
       2024-09-17 11:24:52 +08:00
    @leohuangsulei #5 我之前做的 OA 项目,有些无代码需求,就有形如
    SELECT * FROM ARTICLES WHERE ${columnName} = #{columnValue}
    这样的 SQL
    不过前端传过来的数据都做了过滤
    L0L
        7
    L0L  
       2024-09-17 12:52:28 +08:00
    @BraveXaiver 我理解这种 bug 不能属于 mybatis 或者 mp 的,这个 cve 的漏洞针对的是你的这个软件,你的软件使用 mp 或者 mybatis 导致的问题,修复软件的漏洞,在接口侧做入参校验,是可以规避这种 sql 注入问题。不知道你们的安全扫描是否这样还会扫描,是否能显示修复。本身这个问题细究这个漏洞是不是 mp 或者 mybatis 可能没啥用。
    qq135449773
        9
    qq135449773  
       2024-09-17 17:26:00 +08:00 via Android
    下面的沟通记录截图我越看越想笑。

    我觉得在这个问题上,有问题的不是这个项目本身。
    BraveXaiver
        10
    BraveXaiver  
    OP
       2024-09-17 18:32:21 +08:00
    @qq135449773 #9

    这篇后记的语气也真傲慢,或者说双标。

    引用其原文:

    Mybatis 的${}问题(他们好爱拿这个来举例子)。"外部数据传入 Mybatis ${} 可以造成 SQL 注入" - 这是一个已知的问题(漏洞)。一个应用因为外部不可控参数传入${}并导致 SQL 注入,我们不会去找 Mybatis 给他们提报漏洞(因为对于官方来说,这是一个已知的问题(漏洞),官方给了足够的说明、警告、风险提醒、适用场景以及替代方案),我们会直接找这个应用去提报漏洞

    但是对于 MybatisPlus 来说,几乎没有人知道在使用这个框架的时候要怎么避免 SQL 注入(要不是看了源码,我也不会知道他们什么都不处理就直接做字段拼接),两者根本没有可比性

    然而, 至少今天,不需要查看 MP 的源码,MP 的官方文档: https://baomidou.com/reference/about-cve/

    已经强调:

    该“漏洞”也是前端端传入 SQL 片段 导致 SQL 注入攻击。框架 QueryWrapper UpdateWrapper 字段部分是允许子查询的因此不能人为允许前端传入 SQL 片段。

    如果使用者有这种需求,可以使用 SqlInjectionUtils.check(内容) 或 xxWrapper.checkSqlInjection() 方法来检查,如果检查通过,则不会抛出异常。

    框架也提供了非常严格的条件构造器 LambdaQueryWrapper LambdaUpdateWrapper 推荐使用。
    nyxsonsleep
        11
    nyxsonsleep  
       2024-09-17 20:39:11 +08:00
    @wssy001 #6 前端过滤不就等于不设防吗?
    slowgen
        12
    slowgen  
       2024-09-17 21:03:39 +08:00
    强类型语言的 ORM + 实体类,还能出现列名逃逸,真的挺好笑的。
    你看它那个号称能防御 sql 注入的 SqlInjectionUtils https://github.com/baomidou/mybatis-plus/blob/3.0/mybatis-plus-core/src/main/java/com/baomidou/mybatisplus/core/toolkit/sql/SqlInjectionUtils.java ,碍于网络安全法我就不写绕过细节了,拿去问 AI“你是 sql 注入专家,审计这个 java 代码分析出可以逃逸的情况(比如哪些数据库存在它没提到的关键字)
    ```java
    balabala 代码
    ```


    惊喜连连
    ZeroDu
        13
    ZeroDu  
       2024-09-17 23:40:06 +08:00
    严重怀疑这是有人(同行?)恶意提交的 CVE 。这不明摆着让人换框架吗
    Bingchunmoli
        14
    Bingchunmoli  
       2024-09-18 00:12:19 +08:00 via Android
    @ZeroDu 就是同行,开源中国现在猛猛推新 mybatis 框架。换汤不换药
    xuanbg
        15
    xuanbg  
       2024-09-18 06:13:31 +08:00
    这真是程序设计领域的「因噎废食」,简直笑死
    bli22ard
        16
    bli22ard  
       2024-09-18 09:48:18 +08:00
    外部输入的 sql 参数,不使用预编译参数,不对合法性进行检查,sql 注入了。 这和 mybatis 没什么关系。
    wssy001
        17
    wssy001  
       2024-09-18 10:10:28 +08:00
    @nyxsonsleep #11 前端传过来的数据先是在 gateway 层就做了一层数据过滤 传递到 service 层还有一层数据过滤
    CutieJohn
        18
    CutieJohn  
       2024-09-18 10:51:41 +08:00
    至少两年前就被这个 CVE 搞了,最后我们在内部做了例外
    COOOOOOde
        19
    COOOOOOde  
       2024-09-18 19:52:12 +08:00 via Android
    哪个傻蛋真会这样用啊,知道 mybatis 不就知道这个,我觉得根本不是问题
    cslive
        20
    cslive  
       2024-09-19 10:35:25 +08:00   1
    你这个例子不对
    // #{column}=#{value} ==> `name`="张三" 其中 `name`带转义
    new LambdaUpdateWrapper<user>().eq(true, user::getName, "张三")

    // ${column} = #{value} ==> name = "张三" 其中 name 就是纯字符串拼接
    new UpdateWrapper<user>().eq(true, "name", "张三")
    这个例子才对,mybatis 文档强调过"$"与"#"的区别,还说明"$"是不安全的,但是 mybatis-plus 并没有强调上面两个方法的区别,只是说明两个生成的 sql 相同,没有强调普通 Wrapper 是不安全的,可能存在 sql 注入
    BraveXaiver
        21
    BraveXaiver  
    OP
       2024-09-19 20:06:52 +08:00
    @cslive hmm 所以你同意, 如果官方在文档中强调一下用户需要对用作列名参数的值做检查(就像 mybatis 那样强调),那么便不存在问题吗?
    cslive
        22
    cslive  
       2024-09-20 11:12:41 +08:00
    @BraveXaiver #21 同意,但是官方文档到目前为止没有强调,只有在 cve 界面说了一下
    关于     帮助文档     自助推广系统     博客     API     FAQ     Solana     3174 人在线   最高记录 6679       Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 24ms UTC 12:13 PVG 20:13 LAX 05:13 JFK 08:13
    Do have faith in what you're doing.
    ubao snddm index pchome yahoo rakuten mypaper meadowduck bidyahoo youbao zxmzxm asda bnvcg cvbfg dfscv mmhjk xxddc yybgb zznbn ccubao uaitu acv GXCV ET GDG YH FG BCVB FJFH CBRE CBC GDG ET54 WRWR RWER WREW WRWER RWER SDG EW SF DSFSF fbbs ubao fhd dfg ewr dg df ewwr ewwr et ruyut utut dfg fgd gdfgt etg dfgt dfgd ert4 gd fgg wr 235 wer3 we vsdf sdf gdf ert xcv sdf rwer hfd dfg cvb rwf afb dfh jgh bmn lgh rty gfds cxv xcv xcs vdas fdf fgd cv sdf tert sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf shasha9178 shasha9178 shasha9178 shasha9178 shasha9178 liflif2 liflif2 liflif2 liflif2 liflif2 liblib3 liblib3 liblib3 liblib3 liblib3 zhazha444 zhazha444 zhazha444 zhazha444 zhazha444 dende5 dende denden denden2 denden21 fenfen9 fenf619 fen619 fenfe9 fe619 sdf sdf sdf sdf sdf zhazh90 zhazh0 zhaa50 zha90 zh590 zho zhoz zhozh zhozho zhozho2 lislis lls95 lili95 lils5 liss9 sdf0ty987 sdft876 sdft9876 sdf09876 sd0t9876 sdf0ty98 sdf0976 sdf0ty986 sdf0ty96 sdf0t76 sdf0876 df0ty98 sf0t876 sd0ty76 sdy76 sdf76 sdf0t76 sdf0ty9 sdf0ty98 sdf0ty987 sdf0ty98 sdf6676 sdf876 sd876 sd876 sdf6 sdf6 sdf9876 sdf0t sdf06 sdf0ty9776 sdf0ty9776 sdf0ty76 sdf8876 sdf0t sd6 sdf06 s688876 sd688 sdf86