mybatis-3 icon indicating copy to clipboard operation
mybatis-3 copied to clipboard

add a tag like #{} ${} reopen! i have a new idea!

Open k4n5ha0 opened this issue 4 years ago • 8 comments

Hi! i am a web security engineer. With my communicate, my colleagues accepted use #{} is safe. If don't check code very carefully, useing ${} wil be create a sql injection Also i add some rules into sonarqube to check xml can't have ${} But ordey by and group by must use ${} So I thinging if mybatis can add a tag , something like @{} !{} In this new tag,Mybatis can prevent "sql injection char" likes ' " , * ( ) { } @ nine chars Or In this new tag only accept letters and char _ regex: [a-zA-Z_] So #{} and new tag canbe safe. Only some situation coder will useing ${}. Thx.

k4n5ha0 avatar May 31 '20 01:05 k4n5ha0

Hello @k4n5ha0 ,

Filtering characters can never be a good strategy to prevent SQL injection as there are many techniques to circumvent those filters. The only reliable protection is to use #{}.

For ORDER BY, there usually are several choices, so you may be able to write as follows to avoid ${}.

ORDER BY
<choose>
  <when test="orderBy == 1">
    id desc
  </when>
  <when test="orderBy == 2">
    date_submitted desc
  </when>
  ...

If your app allows end users to supply arbitrary ORDER BY / GROUP BY clause, your job will be very hard...

harawata avatar May 31 '20 09:05 harawata

ok thx very much :-)

k4n5ha0 avatar May 31 '20 09:05 k4n5ha0

Hi!I have a new idea! How about build a safemode like fastjson. url: https://github.com/alibaba/fastjson/wiki/fastjson_safemode

  1. ParserConfig.getGlobalInstance().setSafeMode(true);
  2. fastjson.parser.safeMode=true
  3. -Dfastjson.parser.safeMode=true Three ways to open a safemode.

When Mybatis open this mode, mybatis's $ will throw a Exception. So this mode will prevent Sql Injection! And Forcing programmers use # Thx!

k4n5ha0 avatar Jun 05 '20 14:06 k4n5ha0

${} is an important feature and I am not comfortable with adding an option to disallow its usage. It is not that difficult to use ${} safely, so you should educate your programmers instead of banning it. In case you really have to detect ${}, you may be able to check text nodes in a custom language driver, probably (I do not recommend using it on production for performance reason, though).

harawata avatar Jun 08 '20 14:06 harawata

hi , one of my friend working in a bank , he tells me this bank is using fortify to check mybatis's xml isnt have ${} .i talked with him , he tells me more about mybatis, some company is using "changed mybatis" to reduce sqlinection Possibility, so i wich this feather will be create.secre by deault. thx.

k4n5ha0 avatar Jun 12 '20 00:06 k4n5ha0

Hi,i really thinks create A new tag(for example !{})is a good idea,beacause a tag which: use allowrule regex [a-zA-Z0-9_] or use denyrule in tag string can't include ' " < > % ( ) may help programmers reduce sql injection code.

sql injection with like

Select * from news where title like '%${title}%'

can be

Select * from news where title like '%!{title}%'

sql injection with in

Select * from news where id in (${id})

can be

Select * from news where id in (!{id})

sql injection with order by

Select * from news where title = 'time' order by ${time} asc

can be

Select * from news where title = 'time' order by !{time} asc

this new tag can fix sqlinjection very quickly with lower time.

k4n5ha0 avatar Sep 25 '22 05:09 k4n5ha0

great,We must take security risks more seriously

xiaoqin00 avatar Sep 25 '22 06:09 xiaoqin00

As I wrote in my first comment, filtering characters can never be a good strategy to prevent SQL injection. Trying to filter characters is the opposite of taking the security risk seriously and we will not provide such feature.

harawata avatar Sep 25 '22 11:09 harawata