Skip to content

Commit 7c218b1

Browse files
emacarronemacarron
committed
Fixes #117. Sql injection filter for ${} expressions
1 parent 154f48b commit 7c218b1

File tree

12 files changed

+308
-7
lines changed

12 files changed

+308
-7
lines changed

‎src/main/java/org/apache/ibatis/builder/BaseBuilder.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.Arrays;
1919
import java.util.HashSet;
2020
import java.util.Set;
21+
import java.util.regex.Pattern;
2122

2223
import org.apache.ibatis.mapping.ParameterMode;
2324
import org.apache.ibatis.mapping.ResultSetType;
@@ -45,6 +46,10 @@ public Configuration getConfiguration() {
4546
return configuration;
4647
}
4748

49+
protected Pattern parseExpression(String regex, String defaultValue) {
50+
return Pattern.compile(regex == null ? defaultValue : regex);
51+
}
52+
4853
protected Boolean booleanValueOf(String value, Boolean defaultValue) {
4954
return value == null ? defaultValue : Boolean.valueOf(value);
5055
}

‎src/main/java/org/apache/ibatis/builder/xml/XMLConfigBuilder.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,11 @@ private void settingsElement(XNode context) throws Exception {
217217
configuration.setLogPrefix(props.getProperty("logPrefix"));
218218
configuration.setLogImpl(resolveClass(props.getProperty("logImpl")));
219219
configuration.setConfigurationFactory(resolveClass(props.getProperty("configurationFactory")));
220+
configuration.setInjectionFilterEnabled(booleanValueOf(props.getProperty("injectionFilterEnabled"), false));
221+
configuration.setInjectionFilter(parseExpression(props.getProperty("injectionFilter"), "^[a-zA-Z0-9._]*$"));
220222
}
221223
}
222-
224+
223225
private void environmentsElement(XNode context) throws Exception {
224226
if (context != null) {
225227
if (environment == null) {

‎src/main/java/org/apache/ibatis/scripting/xmltags/TextSqlNode.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,27 @@
1515
*/
1616
package org.apache.ibatis.scripting.xmltags;
1717

18+
import java.util.regex.Pattern;
19+
1820
import org.apache.ibatis.parsing.GenericTokenParser;
1921
import org.apache.ibatis.parsing.TokenHandler;
22+
import org.apache.ibatis.scripting.ScriptingException;
2023
import org.apache.ibatis.type.SimpleTypeRegistry;
2124

2225
/**
2326
* @author Clinton Begin
2427
*/
2528
public class TextSqlNode implements SqlNode {
2629
private String text;
30+
private Pattern injectionFilter;
2731

2832
public TextSqlNode(String text) {
33+
this(text, null);
34+
}
35+
36+
public TextSqlNode(String text, Pattern injectionFilter) {
2937
this.text = text;
38+
this.injectionFilter = injectionFilter;
3039
}
3140

3241
public boolean isDynamic() {
@@ -37,7 +46,7 @@ public boolean isDynamic() {
3746
}
3847

3948
public boolean apply(DynamicContext context) {
40-
GenericTokenParser parser = createParser(new BindingTokenParser(context));
49+
GenericTokenParser parser = createParser(new BindingTokenParser(context, injectionFilter));
4150
context.appendSql(parser.parse(text));
4251
return true;
4352
}
@@ -49,9 +58,11 @@ private GenericTokenParser createParser(TokenHandler handler) {
4958
private static class BindingTokenParser implements TokenHandler {
5059

5160
private DynamicContext context;
61+
private Pattern injectionFilter;
5262

53-
public BindingTokenParser(DynamicContext context) {
63+
public BindingTokenParser(DynamicContext context, Pattern injectionFilter) {
5464
this.context = context;
65+
this.injectionFilter = injectionFilter;
5566
}
5667

5768
public String handleToken(String content) {
@@ -62,10 +73,18 @@ public String handleToken(String content) {
6273
context.getBindings().put("value", parameter);
6374
}
6475
Object value = OgnlCache.getValue(content, context.getBindings());
65-
return (value == null ? "" : String.valueOf(value)); // issue #274 return "" instead of "null"
76+
String srtValue = (value == null ? "" : String.valueOf(value)); // issue #274 return "" instead of "null"
77+
checkInjection(srtValue);
78+
return srtValue;
6679
}
67-
}
6880

81+
private void checkInjection(String value) {
82+
if (injectionFilter != null && !injectionFilter.matcher(value).matches()) {
83+
throw new ScriptingException("Invalid input. Please conform to regex" + injectionFilter.pattern());
84+
}
85+
}
86+
}
87+
6988
private static class DynamicCheckerTokenParser implements TokenHandler {
7089

7190
private boolean isDynamic;

‎src/main/java/org/apache/ibatis/scripting/xmltags/XMLLanguageDriver.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
package org.apache.ibatis.scripting.xmltags;
1717

18+
import java.util.regex.Pattern;
19+
1820
import org.apache.ibatis.builder.xml.XMLMapperEntityResolver;
1921
import org.apache.ibatis.executor.parameter.ParameterHandler;
2022
import org.apache.ibatis.mapping.BoundSql;
@@ -48,13 +50,17 @@ public SqlSource createSqlSource(Configuration configuration, String script, Cla
4850
return createSqlSource(configuration, parser.evalNode("/script"), parameterType);
4951
} else {
5052
script = PropertyParser.parse(script, configuration.getVariables()); // issue #127
51-
TextSqlNode textSqlNode = new TextSqlNode(script);
53+
TextSqlNode textSqlNode = new TextSqlNode(script, getInjectionFilter(configuration));
5254
if (textSqlNode.isDynamic()) {
5355
return new DynamicSqlSource(configuration, textSqlNode);
5456
} else {
5557
return new RawSqlSource(configuration, script, parameterType);
5658
}
5759
}
5860
}
61+
62+
private Pattern getInjectionFilter(Configuration configuration) {
63+
return configuration.isInjectionFilterEnabled() ? configuration.getInjectionFilter() : null;
64+
}
5965

6066
}

‎src/main/java/org/apache/ibatis/scripting/xmltags/XMLScriptBuilder.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.HashMap;
2020
import java.util.List;
2121
import java.util.Map;
22+
import java.util.regex.Pattern;
2223

2324
import org.apache.ibatis.builder.BaseBuilder;
2425
import org.apache.ibatis.builder.BuilderException;
@@ -67,7 +68,7 @@ private List<SqlNode> parseDynamicTags(XNode node) {
6768
XNode child = node.newXNode(children.item(i));
6869
if (child.getNode().getNodeType() == Node.CDATA_SECTION_NODE || child.getNode().getNodeType() == Node.TEXT_NODE) {
6970
String data = child.getStringBody("");
70-
TextSqlNode textSqlNode = new TextSqlNode(data);
71+
TextSqlNode textSqlNode = new TextSqlNode(data, getInjectionFilter(configuration));
7172
if (textSqlNode.isDynamic()) {
7273
contents.add(textSqlNode);
7374
isDynamic = true;
@@ -86,6 +87,10 @@ private List<SqlNode> parseDynamicTags(XNode node) {
8687
}
8788
return contents;
8889
}
90+
91+
private Pattern getInjectionFilter(Configuration configuration) {
92+
return configuration.isInjectionFilterEnabled() ? configuration.getInjectionFilter() : null;
93+
}
8994

9095
private Map<String, NodeHandler> nodeHandlers = new HashMap<String, NodeHandler>() {
9196
private static final long serialVersionUID = 7123056019193266281L;

‎src/main/java/org/apache/ibatis/session/Configuration.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Map;
2525
import java.util.Properties;
2626
import java.util.Set;
27+
import java.util.regex.Pattern;
2728

2829
import org.apache.ibatis.binding.MapperRegistry;
2930
import org.apache.ibatis.builder.CacheRefResolver;
@@ -103,6 +104,9 @@ public class Configuration {
103104
protected boolean useColumnLabel = true;
104105
protected boolean cacheEnabled = true;
105106
protected boolean callSettersOnNulls = false;
107+
protected boolean injectionFilterEnabled = false;
108+
protected Pattern injectionFilter = Pattern.compile("^[a-zA-Z0-9._]*$");
109+
106110
protected String logPrefix;
107111
protected Class <? extends Log> logImpl;
108112
protected LocalCacheScope localCacheScope = LocalCacheScope.SESSION;
@@ -673,6 +677,22 @@ public void addCacheRef(String namespace, String referencedNamespace) {
673677
cacheRefMap.put(namespace, referencedNamespace);
674678
}
675679

680+
public boolean isInjectionFilterEnabled() {
681+
return injectionFilterEnabled;
682+
}
683+
684+
public void setInjectionFilterEnabled(boolean injectionFilterEnabled) {
685+
this.injectionFilterEnabled = injectionFilterEnabled;
686+
}
687+
688+
public Pattern getInjectionFilter() {
689+
return injectionFilter;
690+
}
691+
692+
public void setInjectionFilter(Pattern injectionFilter) {
693+
this.injectionFilter = injectionFilter;
694+
}
695+
676696
/*
677697
* Parses all the unprocessed statement nodes in the cache. It is recommended
678698
* to call this method once all the mappers are added as it provides fail-fast

‎src/test/java/org/apache/ibatis/submitted/injection/CreateDB.sql

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--
2+
-- Copyright 2009-2012 the original author or authors.
3+
--
4+
-- Licensed under the Apache License, Version 2.0 (the "License");
5+
-- you may not use this file except in compliance with the License.
6+
-- You may obtain a copy of the License at
7+
--
8+
-- http://www.apache.org/licenses/LICENSE-2.0
9+
--
10+
-- Unless required by applicable law or agreed to in writing, software
11+
-- distributed under the License is distributed on an "AS IS" BASIS,
12+
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
-- See the License for the specific language governing permissions and
14+
-- limitations under the License.
15+
--
16+
17+
drop table users if exists;
18+
19+
create table users (
20+
id int,
21+
name varchar(20)
22+
);
23+
24+
insert into users (id, name) values(1, 'User1');
25+
insert into users (id, name) values(2, 'User2');

‎src/test/java/org/apache/ibatis/submitted/injection/InjectionTest.java

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright 2009-2013 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.apache.ibatis.submitted.injection;
17+
18+
import java.io.Reader;
19+
import java.sql.Connection;
20+
import java.util.List;
21+
import java.util.regex.Pattern;
22+
23+
import org.apache.ibatis.exceptions.PersistenceException;
24+
import org.apache.ibatis.io.Resources;
25+
import org.apache.ibatis.jdbc.ScriptRunner;
26+
import org.apache.ibatis.session.SqlSession;
27+
import org.apache.ibatis.session.SqlSessionFactory;
28+
import org.apache.ibatis.session.SqlSessionFactoryBuilder;
29+
import org.junit.Assert;
30+
import org.junit.BeforeClass;
31+
import org.junit.Test;
32+
33+
public class InjectionTest {
34+
35+
private static SqlSessionFactory sqlSessionFactory;
36+
37+
@BeforeClass
38+
public static void setUp() throws Exception {
39+
// create an SqlSessionFactory
40+
Reader reader = Resources.getResourceAsReader("org/apache/ibatis/submitted/injection/mybatis-config.xml");
41+
sqlSessionFactory = new SqlSessionFactoryBuilder().build(reader);
42+
reader.close();
43+
44+
// populate in-memory database
45+
SqlSession session = sqlSessionFactory.openSession();
46+
Connection conn = session.getConnection();
47+
reader = Resources.getResourceAsReader("org/apache/ibatis/submitted/injection/CreateDB.sql");
48+
ScriptRunner runner = new ScriptRunner(conn);
49+
runner.setLogWriter(null);
50+
runner.runScript(reader);
51+
reader.close();
52+
session.close();
53+
}
54+
55+
@Test
56+
public void shouldGetAUser() {
57+
SqlSession sqlSession = sqlSessionFactory.openSession();
58+
try {
59+
Mapper mapper = sqlSession.getMapper(Mapper.class);
60+
List<User> users = mapper.getUsers("users");
61+
Assert.assertEquals(1, users.size());
62+
} finally {
63+
sqlSession.close();
64+
}
65+
}
66+
67+
@Test(expected=PersistenceException.class)
68+
public void shouldDetectAnSqlInjectionAttack() {
69+
SqlSession sqlSession = sqlSessionFactory.openSession();
70+
try {
71+
Mapper mapper = sqlSession.getMapper(Mapper.class);
72+
List<User> users = mapper.getUsers("users where 1=1 union select * from users");
73+
Assert.assertEquals(1, users.size());
74+
} finally {
75+
sqlSession.close();
76+
}
77+
}
78+
79+
@Test
80+
public void check() {
81+
Pattern p = Pattern.compile("^[a-zA-Z0-9.]*$");
82+
Assert.assertTrue(p.matcher("a9.").matches());
83+
}
84+
85+
86+
}

‎src/test/java/org/apache/ibatis/submitted/injection/Mapper.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright 2009-2012 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.apache.ibatis.submitted.injection;
17+
18+
import java.util.List;
19+
20+
public interface Mapper {
21+
22+
List<User> getUsers(String name);
23+
24+
}

‎src/test/java/org/apache/ibatis/submitted/injection/Mapper.xml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
Copyright 2009-2013 the original author or authors.
4+
5+
Licensed under the Apache License, Version 2.0 (the "License");
6+
you may not use this file except in compliance with the License.
7+
You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
-->
17+
<!DOCTYPE mapper
18+
PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN"
19+
"http://mybatis.org/dtd/mybatis-3-mapper.dtd">
20+
21+
<mapper namespace="org.apache.ibatis.submitted.injection.Mapper">
22+
23+
<select id="getUsers" resultType="org.apache.ibatis.submitted.injection.User">
24+
select * from ${value} where id = 1
25+
</select>
26+
27+
</mapper>

0 commit comments

Comments
 (0)