Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

close issues #6167

Closed
cangkuai opened this issue May 15, 2024 · 16 comments
Closed

close issues #6167

cangkuai opened this issue May 15, 2024 · 16 comments

Comments

@cangkuai
Copy link

cangkuai commented May 15, 2024

No description provided.

@cangkuai cangkuai changed the title update CRUD可能存在布尔盲注漏洞 Possible Boolean Blind Vulnerability in update CRUD May 15, 2024
@VampireAchao
Copy link
Contributor

不要这么用

@miemieYaho
Copy link
Member

笑死个仙人了

@cangkuai cangkuai changed the title Possible Boolean Blind Vulnerability in update CRUD close issues May 29, 2024
@jinceon
Copy link

jinceon commented May 29, 2024

顺着公众号的声明来的,特意来看看这个issue怎么回事。
说实话,一开始我的观点是赞同mybatis-plus声明里评论的观点的。
就好比 mybatis 的 #{}${} ,你不能用了 ${} 还要从前端传入任意参数。

但是看到楼上2个回复的用户还挂着 contributor 和 member 的徽章,我觉得你们官方的问题更大。

评论1的问题:网友竟然来提问了,大概率就是个小白,你回复个【不要这么用】还不如别回复。
可以回复一句,就好比mybatis的#{}${} ,mybatis-plus提供了 LambdaUpdateWrapper 和 UpdateWrapper 。
而且这么多的issue里类似的回复不是一次两次,有些issue是直接关闭,你们是有issue的kpi考核吗?

// #{column}=#{value}      ==>      `name`="张三"          其中 `name`带转义
new LambdaUpdateWrapper<user>().eq(true, user::getName, "张三")  

//  ${column} = #{value}    ==>    name = "张三"   其中name就是纯字符串拼接
new UpdateWrapper<user>().eq(true, "name", "张三")

其实说白了就是mybatis-plus官方假定了你使用UpdateWrapper的时候,一定是后端写死的column的。
但是它又不像mybatis那样反复强调 # 和 $的区别。
所以拿这个来和mybatis举例也不恰当,因为几乎所有人都知道#和$的区别。
但是出于目前框架几乎都处理了sql injection的时代背景,有人不理解为什么没对column做处理很正常。

评论2的问题:【笑死个仙人了】我都不想吐槽了。
如果你只是个网友,回复了就回复了,好歹挂着个【Member】的徽章,一点影响都不注意的吗?

@miemieYaho
Copy link
Member

注意什么?
作为一个合格的开发column还能前端传什么你都直接用,那更适合去工地搬砖,搬砖几乎不需要动脑子

@jinceon
Copy link

jinceon commented May 29, 2024 via email

@lesper
Copy link

lesper commented Jun 3, 2024

嗯,框架就不该解决sql注入。 毕竟作为一个合格的开发,连sql注入这种低级问题都不能解决,就应该去搬砖,搬砖不用脑子。 发自我的iPhone

------------------ Original ------------------ From: miemieYaho @.> Date: Wed,May 29,2024 5:34 PM To: baomidou/mybatis-plus @.> Cc: JinCeon @.>, Comment @.> Subject: Re: [baomidou/mybatis-plus] close issues (Issue #6167) 注意什么? 作为一个合格的开发column还能前端传什么你都直接用,那更适合去工地搬砖,搬砖几乎不需要动脑子 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

《合格的开发》《前端传什么用什么》要不后端直接引入apijson或者开个sql执行接口,前端想要什么数据自己写SQL,出了问题也是前端的。

@lly-ke
Copy link

lly-ke commented Jun 3, 2024

#1207

和我之前提的类似, 3.5.3.2 加了checkSqlInjection, 只解决了QueryWrapper 中的sql注入, 其他类使用姿势不会还是会被sql注入

既然这个框架不全局处理sql注入, 加checkSqlInjection有什么意义呢???

@floydmd2x
Copy link

@miemieYaho 你的头像很符合我对你的第一感觉, 眼神里面充满睿智

@miemieYaho
Copy link
Member

@miemieYaho 你的头像很符合我对你的第一感觉, 眼神里面充满睿智

拿头像说事你他娘的还真是个人才

@VampireAchao
Copy link
Contributor

顺着公众号的声明来的,特意来看看这个issue怎么回事。 说实话,一开始我的观点是赞同mybatis-plus声明里评论的观点的。 就好比 mybatis 的 #{}${} ,你不能用了 ${} 还要从前端传入任意参数。

但是看到楼上2个回复的用户还挂着 contributor 和 member 的徽章,我觉得你们官方的问题更大。

评论1的问题:网友竟然来提问了,大概率就是个小白,你回复个【不要这么用】还不如别回复。 可以回复一句,就好比mybatis的#{}${} ,mybatis-plus提供了 LambdaUpdateWrapper 和 UpdateWrapper 。 而且这么多的issue里类似的回复不是一次两次,有些issue是直接关闭,你们是有issue的kpi考核吗?

// #{column}=#{value}      ==>      `name`="张三"          其中 `name`带转义
new LambdaUpdateWrapper<user>().eq(true, user::getName, "张三")  

//  ${column} = #{value}    ==>    name = "张三"   其中name就是纯字符串拼接
new UpdateWrapper<user>().eq(true, "name", "张三")

其实说白了就是mybatis-plus官方假定了你使用UpdateWrapper的时候,一定是后端写死的column的。 但是它又不像mybatis那样反复强调 # 和 $的区别。 所以拿这个来和mybatis举例也不恰当,因为几乎所有人都知道#和$的区别。 但是出于目前框架几乎都处理了sql injection的时代背景,有人不理解为什么没对column做处理很正常。

评论2的问题:【笑死个仙人了】我都不想吐槽了。 如果你只是个网友,回复了就回复了,好歹挂着个【Member】的徽章,一点影响都不注意的吗?

不要这么讲

【Member】的徽章,只代表持续对该项目的贡献得到了认可,同时也给予维护开源项目代码、进行社区issue回复的责任,这并不意味着可以被道德绑架要求每一次回复都“尽善尽美”。做开源完全是“为爱发电”,但最近越来越让我感觉像是在做服务行业。开源本身是一件伟大的事,将自己的经验所得,分享给全世界,并且接受让所有人看到、参与的义举。如果您觉得我回复的简短,实际上我们已经反复、再三强调如何处理、预防该问题。甚至单独拿出一页官方文档说明:
https://baomidou.com/reference/about-cve/

如果您觉得我个人德不配位,我虚心接受,尽量以后在回复您的issue时三思而后行,也感谢您对MP的关注,和指出我个人的不足

这里我对您衷心道歉,也希望大家引以为戒,不要再像我一样,在为生计奔波的同时,抽出个人时间用爱发电,还不认真发电

@jinceon
Copy link

jinceon commented Jun 4, 2024

@VampireAchao

尽量以后在回复您的issue时三思而后行

我都不知道该说啥了,发个狗头表情🐶保命

@jinceon
Copy link

jinceon commented Jun 4, 2024

好多人还是拿其他orm和前端传参来说事。

我想再说下文档。

就拿mybatis的文档来说,官方非常清晰地介绍了 #{} 和 ${} 的使用场景。

你会非常清晰地理解两者的区别。

mybatis – MyBatis 3 | XML 映射器
image
但是mybatis-plus呢?你完全感受不到普通wrapper和lambda wrapper的差异。

它还特意告诉你,普通wrapper和lambda wrapper生成的sql相同。

QueryWrapper<User> queryWrapper = new QueryWrapper<>();
queryWrapper.eq("name", "老王");

对于用户来说,"name"和"老王"都是字符串,结果底层实现的时候"name"是字符串拼接,"老王"却是做了处理的。

谁能想到?

因为在用mybatis的时候,#{}和${}是用户自己写的,而用mybatis-plus的queryWrapper的时候,相当于mybatis-plus替用户写了#{}和${},mybatis-plus对用户屏蔽了细节。

如果由我设计,我会类似mybatis一样,强烈推荐用User::getName来做column字段,这样也可以在字段名重命名的时候立马就发现了,方便全局重构。

用"name"这种字符串唯一的场景,就是需要(前端)动态传入。

image

@iseki0
Copy link

iseki0 commented Jun 6, 2024

提供这个 API 是没问题的,但 MyBatisPlus 官方应该注意下自己的文档质量问题,至少所有的 public 成员都要有 API 文档。
这个 API 是很有必要的,举个例子 eq("to_lower(username)", username) 这里就是要拼接 SQL。不是说不能有一个 Clause.toLower(User::username) 这样的东西,而是说在大多数情况下,这样成本有点高。
至于有人指出的在方法名中加 unsafe 这不可取,在到处都是 unsafe 的情况下,增加这个描述只会造成视觉污染,无益于解决问题。

@lghcode
Copy link

lghcode commented Jun 6, 2024

你们有人利用这个SQL注入成功删除数据么,我没复现出来

@leetom
Copy link

leetom commented Jul 12, 2024

笑死个仙人了

你这个沙雕终于被人挂了
原来不是MP有问题,只是因为到处都有这老鼠屎

@miemieYaho
Copy link
Member

笑死个仙人了

你这个沙雕终于被人挂了 原来不是MP有问题,只是因为到处都有这老鼠屎

你个臭傻逼在说什么jb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

11 participants