abel533 / Mapper

Mybatis Common Mapper - Easy to use
https://mybatis.io
MIT License
7.34k stars 1.63k forks source link

selectByIds function sql injection #862

Open hldfight opened 2 years ago

hldfight commented 2 years ago

Write the following test demo: 1、UserController.java: @Controller public class UserController {

@Autowired
UserService userService;

@RequestMapping("gets")
@ResponseBody
public List<User> getUser(String ids) {
    List<String> idList = Arrays.asList(ids.split(","));

    return userService.gets(idList);
}

} 2、UserService.java: @Service public interface UserService {

List<User> gets(Collection<String> ids);

} 3、UserServiceImpl.java: @Service public class UserServiceImpl implements UserService {

@Autowired
UserMapper userMapper;

@Override
public List<User> gets(Collection<String> ids) {
    if (ids == null || ids.isEmpty())
        return new ArrayList<>();
    String concatIds = StringUtils.concat(ids, "'", ",");
    return (List<User>) userMapper.selectByIds(concatIds);
}

} 4、UserMapper.java: @org.apache.ibatis.annotations.Mapper public interface UserMapper extends Mapper, MySqlMapper, IdsMapper {

} 5、Access the /gets route in the above demo for sql injection attack: (1)Under normal circumstances, when the ids parameter value is passed in 1, 2, the data with id 1 and 2 can be obtained: image (2)But when the ids parameter value is 1') or 1=1-- -, you can get all the data in the database: image

abel533 commented 2 years ago

At the business level, check according to the type of ID.

hldfight commented 2 years ago

Maybe developers won't notice this problem, I believe that many developers do not verify whether the id parameter value is valid, which will lead to many vulnerabilities. the best solution is to use parameterized query for ids in selectByIds function, that is, use #{} symbol to construct sql statement. This makes applications using the tk.mybatis framework more secure.

------------------ 原始邮件 ------------------ 发件人: "abel533/Mapper" @.>; 发送时间: 2022年7月20日(星期三) 下午5:04 @.>; @.**@.>; 主题: Re: [abel533/Mapper] selectByIds function sql injection (Issue #862)

At the business level, check according to the type of ID.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

abel533 commented 2 years ago

Guaranteed compatibility. Have a PR?