From 4cd2af10499ac6dc4f82bda179d9f414a522abef Mon Sep 17 00:00:00 2001 From: cdmikechen Date: Sat, 7 Jan 2023 09:44:20 +0800 Subject: [PATCH] SUBMARINE-1361. Fix Submarine SQL injection vulnerability ### What is this PR for? Currently a SQL injection vulnerability has been checked in submarine and the relevant part of the `like` statement in mybatis needs to be fixed. ### What type of PR is it? Bug Fix ### Todos * [x] - replace `like` statement to `concat('%', #{param}, '%')` ### What is the Jira issue? https://issues.apache.org/jira/browse/SUBMARINE-1361 ### How should this be tested? Added a test case verification code in `submarine-server/server-database/src/test/java/org/apache/submarine/server/database/workbench/database/service/SysUserServiceTest.java` ### Screenshots (if appropriate) NA ### Questions: * Do the license files need updating? No * Are there breaking changes for older versions? No * Does this need new documentation? No Author: cdmikechen Signed-off-by: cdmikechen Closes #1037 from cdmikechen/SUBMARINE-1361 and squashes the following commits: 34fb34b6 [cdmikechen] Avoid sql injection --- .../submarine/database/mappers/SysDeptMapper.xml | 4 ++-- .../database/mappers/SysDictItemMapper.xml | 4 ++-- .../submarine/database/mappers/SysDictMapper.xml | 4 ++-- .../submarine/database/mappers/SysUserMapper.xml | 4 ++-- .../database/service/SysUserServiceTest.java | 13 +++++++++++++ 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDeptMapper.xml b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDeptMapper.xml index a11ee5195a..e98d503b7d 100644 --- a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDeptMapper.xml +++ b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDeptMapper.xml @@ -42,8 +42,8 @@ SELECT a.*, b.dept_name AS parent_name FROM sys_department a LEFT JOIN sys_department b ON a.parent_code=b.dept_code WHERE 1=1 - AND a.`dept_code` like '%${deptCode}%' - AND a.`dept_name` like '%${deptName}%' + AND a.`dept_code` like concat('%', #{deptCode}, '%') + AND a.`dept_name` like concat('%', #{deptName}, '%') ORDER BY a.sort_order diff --git a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictItemMapper.xml b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictItemMapper.xml index 731bb700b3..55150e720c 100644 --- a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictItemMapper.xml +++ b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictItemMapper.xml @@ -31,8 +31,8 @@ diff --git a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictMapper.xml b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictMapper.xml index 55db3a9b09..69e5de1b4e 100644 --- a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictMapper.xml +++ b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysDictMapper.xml @@ -31,8 +31,8 @@ diff --git a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysUserMapper.xml b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysUserMapper.xml index 49c4e9ec79..c24ad71e61 100644 --- a/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysUserMapper.xml +++ b/submarine-server/server-database/src/main/resources/org/apache/submarine/database/mappers/SysUserMapper.xml @@ -39,8 +39,8 @@ SELECT a.*, b.dept_name FROM sys_user a LEFT JOIN sys_department b ON a.dept_code = b.dept_code WHERE 1 = 1 AND a.`dept_code` = #{deptCode} - AND a.`user_name` like '%${userName}%' - AND a.`email` like '%${email}%' + AND a.`user_name` like concat('%', #{userName}, '%') + AND a.`email` like concat('%', #{email}, '%') ORDER BY a.create_time diff --git a/submarine-server/server-database/src/test/java/org/apache/submarine/server/database/workbench/database/service/SysUserServiceTest.java b/submarine-server/server-database/src/test/java/org/apache/submarine/server/database/workbench/database/service/SysUserServiceTest.java index bbeb4aceb0..f3fbc12963 100644 --- a/submarine-server/server-database/src/test/java/org/apache/submarine/server/database/workbench/database/service/SysUserServiceTest.java +++ b/submarine-server/server-database/src/test/java/org/apache/submarine/server/database/workbench/database/service/SysUserServiceTest.java @@ -78,6 +78,19 @@ public void addUserTest() throws Exception { 10); LOG.debug("userList.size():{}", userList.size()); assertEquals(userList.size(), 1); + + // Avoid sql injection. + // Issue: https://issues.apache.org/jira/browse/SUBMARINE-1361 + List sqlInjectTestList = userService.queryPageList( + String.format("%s' or 1=1 or 1='", sysUser.getUserName()), + null, + null, + null, + null, + 0, + 10); + assertEquals("SQL Injection Vulnerability Detected!", sqlInjectTestList.size(), 0); + SysUserEntity user = userList.get(0); assertEquals(sysUser.getEmail(), user.getEmail());