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

select *的规则检查有bug #10

Open
index3389 opened this issue Sep 3, 2019 · 16 comments
Open

select *的规则检查有bug #10

index3389 opened this issue Sep 3, 2019 · 16 comments
Labels
bug Something isn't working

Comments

@index3389
Copy link

非常感谢博主的速度,这么快就增加了select *的规则检查。

今天体验了一下,发现有bug。

select order_id,user_id,order_amount * discount as product_amount from orders where user_id=#{user_id} and status=1 and is_del=false

这段SQL被插件扫描出来了,里面有*,但是并不是查询所有的字段。

@index3389
Copy link
Author

index3389 commented Sep 3, 2019

当然了,如果从另外一个角度看,SQL里面不能有任何业务逻辑(业务逻辑应该放在程序里面,以便系统的扩展)。扫描出来,也是没有问题的。

算不算一个问题,看博主的权衡了

@index3389
Copy link
Author

select order_id,user_id,order_amount from orders where user_id=#{user_id} and status=1 and is_del=false and available_amount > order_amount*0.05

这种也会扫描出来

@donhui
Copy link
Owner

donhui commented Sep 4, 2019

当然了,如果从另外一个角度看,SQL里面不能有任何业务逻辑(业务逻辑应该放在程序里面,以便系统的扩展)。扫描出来,也是没有问题的。

算不算一个问题,看博主的权衡了

这里的 * 是乘法的意思了

@donhui
Copy link
Owner

donhui commented Sep 4, 2019

这个情况 还没想好怎么规避

@donhui donhui added the bug Something isn't working label Sep 4, 2019
@snoopyhzy
Copy link

自己其实也一直想写个不依赖于SONAR的MYBATIS或IBATIS SQL分析器,包括去支持SQL注入风险检测(主要是$替换在where中使用,in ('') 和 函数出现这种场景多一点),主要是个人不太会写XML读取一直没有找到一个好点的反向MYBATIS XML为SQLID对照关系的开源代码。

这个其实感觉和博主没引入任何一个SQL解析器通过生成SQL的AST语法树有关系。
个人觉得可以尝试使用druid的sqlparser进行这个事情,但这个可能也没有行号,博主通过内容去匹配行号,在小的规范的项目里可能还没什么,我们的项目一个XML里有几千个语句可能就不适合了。

如果通过AST去处理这个事情,那参数里可能就要加数据库类型,比如MYSQL还是ORACLE还是PG之类的了。

当然引入AST后可以基于SQL语法做更多的判定,包括性能判定。
比如博主认为1=1是个风险点,我们内部也这样查过,但不能用1等于1,需要判定恒等,否则很大程度上开发会通过2=2来规避这个规则。并且1=1是一个拼接条件表达式的必要内容,用这个判定风险,误报的概率可能会非常大,比较合适的做法是通过AST语法树,判定WHERE 节点中,非可选的是否只有一个恒等表达式。

就性能上,一些做法的参考,包括
1.纯基于语法,比如去判定,是否存在笛卡尔积。
2.通过JDBC关联一下数据库,获取一些METADATA,包括索引/索引字段信息,去判定OTLP语句有没有正确使用索引等等。

但感觉这些就要对这个工具做比较大的改造了,毕竟基于AST和基于纯文本的扫描规则,差异还是很大的。

@donhui
Copy link
Owner

donhui commented Sep 29, 2019

@snoopyhzy
非常赞同你的这些想法
1、匹配行号在 Mapper 文件比较大时,会出现性能问题
2、AST 我们也曾内部讨论过,目前也还停留在理论层面,我们还没有进行相关实践
3、关于连接数据库,连接不同环境的数据库(数据量不一样),可能会有不同的结果
4、相对对这个工具做大的改造,我更倾向与写一个通用的工具,而不是 SonarQube 插件

再次谢谢你~~

@snoopyhzy
Copy link

@donghui AST的话,相关的内容我已经内部实践过了,也自己查过mybatis的了,但mybatis有一些语法要完全从XML还原为SQL还是很麻烦的,比如trim标签/if标签等等。
因为公司的一些要求的原因不能开源。
用的druid的ast,写法在官方都有说明 https://github.com/snoopyhzy/druid-sql-ast-gui

是直接用visit访问,每个检查规则都继承了vistor很方便的 https://github.com/alibaba/druid/wiki/SQL_Parser_Demo_visitor
然后用java spi或者其他方法来注册规则,以实现规则的动态配置。

我自己也写了个小工具来快速的查看AST语法树的结构,小工具是我非工作时间开发的,所以可以开源,https://github.com/snoopyhzy/druid-sql-ast-gui,找到语法树对应的节点,override一下visit方法,把具体的结果加到结果报告里就好了。

@donhui
Copy link
Owner

donhui commented Nov 26, 2019

@snoopyhzy 谢谢你的反馈,我学习下~~

@snoopyhzy
Copy link

@snoopyhzy 谢谢你的反馈,我学习下~~

其实关注到您这个项目主要是想找个MYBATIS的解析插件把行号和对应的SQL解析出来。

fock了一下,我来试试改,加AST,做点共享,最近写的多了有感觉,刚刚开发把内部的检查工具转SONAR插件了,被仓库KEY冲突坑了好久。- -邮件您咨询也没啥反应哈哈。。

@donhui
Copy link
Owner

donhui commented Nov 29, 2019

@snoopyhzy
前几天比较忙把邮件提醒给关掉了,刚打开,哈哈
一种插件只能支持一种语言的
它是根据文件后缀判断是哪种语言的
一个文件貌似也只能被一种语言的插件解析

@snoopyhzy
Copy link

@snoopyhzy
前几天比较忙把邮件提醒给关掉了,刚打开,哈哈
一种插件只能支持一种语言的
它是根据文件后缀判断是哪种语言的
一个文件貌似也只能被一种语言的插件解析

不是,一个插件可以多语言的,我已经成了,我的问题是仓库的KEY重了。一开始没注意到web.log,只看了sonar.log。

@donhui
Copy link
Owner

donhui commented Nov 29, 2019

@snoopyhzy
前几天比较忙把邮件提醒给关掉了,刚打开,哈哈
一种插件只能支持一种语言的
它是根据文件后缀判断是哪种语言的
一个文件貌似也只能被一种语言的插件解析

不是,一个插件可以多语言的,我已经成了,我的问题是仓库的KEY重了。一开始没注意到web.log,只看了sonar.log。

嗯,仓库的 KEY 这个我这边是 git 仓库的 namespace+project_name ,这样就不会重复了

@snoopyhzy
Copy link

snoopyhzy commented Nov 29, 2019

@donhui 就这个ISSUE做了个DEMO供您参考,SQL语言没指定就是DRUID的通用解释器,具体应该配到SONAR中应该配变量,入口类参考Checker实际上那个SQL嵌入到您那个扫描器的matchRuleAndSaveIssue方法里就可用了。

snoopyhzy@671df82

因为做的最简单的,结果是
[Result{obj=*, rule='NoUseSelectAllColumnsRule'}]

Process finished with exit code 0

@LinWanCen
Copy link

LinWanCen commented Jul 8, 2020

https://github.com/gretard/sonar-sql-plugin

这个插件里实现了,而且他的描述比较美观,可以参考

image

@donhui
Copy link
Owner

donhui commented Jul 8, 2020

@LinWanCen 感谢,有空我研究下~~

@snoopyhzy
Copy link

https://github.com/gretard/sonar-sql-plugin

这个插件里实现了,而且他的描述比较美观,可以参考

image

哇,自从你离开sdc以后,很久没看到你在wiki更新了,没想到还能在github看到

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants