Skip to content

Commit 6fba04f

Browse files
author
jlacey@google.com
committed
Escape username and domain values to avoid SQL injection in string
concatenation (b/13399766, 2 of 2). Code review: https://codereview.appspot.com/75770044
1 parent 6c98659 commit 6fba04f

File tree

4 files changed

+149
-2
lines changed

4 files changed

+149
-2
lines changed

Diff for: projects/dctm-core/source/java/com/google/enterprise/connector/dctm/DctmAuthenticationManager.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,15 @@ private String getUserName(ISessionManager sessionManager,
139139
StringBuilder queryBuff = new StringBuilder();
140140
queryBuff.append("select user_name, user_ldap_dn from ");
141141
queryBuff.append("dm_user where user_login_name = '");
142-
queryBuff.append(userLoginName);
142+
queryBuff.append(DqlUtils.escapeString(userLoginName));
143143
if (!domainName.isEmpty()) {
144144
queryBuff.append("' and user_source = 'LDAP'");
145145
queryBuff.append(" and LOWER(user_ldap_dn) like '%,");
146-
queryBuff.append(domainName);
146+
queryBuff.append(DqlUtils.escapePattern(domainName.toString(), '\\'));
147147
if (domainName.size() == 1) { // NetBIOS domain
148148
queryBuff.append(",%");
149149
}
150+
queryBuff.append("' escape '\\");
150151
}
151152
queryBuff.append("'");
152153

Diff for: projects/dctm-core/source/java/com/google/enterprise/connector/dctm/DqlUtils.java

+50
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,56 @@ public static void appendObjectTypes(StringBuilder buffer,
5757
buffer.append(')');
5858
}
5959

60+
/**
61+
* Escapes single quotes (') in the given value by doubling them,
62+
* as per SQL.
63+
*/
64+
public static String escapeString(String value) {
65+
StringBuilder buffer = new StringBuilder(value.length() + 2);
66+
for (int i = 0; i < value.length(); i++) {
67+
char c = value.charAt(i);
68+
switch (c) {
69+
case '\'':
70+
buffer.append("''");
71+
break;
72+
default:
73+
buffer.append(c);
74+
break;
75+
}
76+
}
77+
return buffer.toString();
78+
}
79+
80+
/**
81+
* Escapes single quotes (') in the given value by doubling them, as
82+
* per SQL, and escapes the LIKE wildcards, percent (%) and
83+
* underscore (_), using the given escape character, as well as
84+
* escaping the escape character itself.
85+
*/
86+
public static String escapePattern(String value, char escapeChar) {
87+
StringBuilder buffer = new StringBuilder(value.length() + 2);
88+
for (int i = 0; i < value.length(); i++) {
89+
char c = value.charAt(i);
90+
switch (c) {
91+
case '\'':
92+
buffer.append("''");
93+
break;
94+
case '%':
95+
case '_':
96+
buffer.append(escapeChar);
97+
buffer.append(c);
98+
break;
99+
default:
100+
if (c == escapeChar) {
101+
buffer.append(escapeChar);
102+
}
103+
buffer.append(c);
104+
break;
105+
}
106+
}
107+
return buffer.toString();
108+
}
109+
60110
/** This class should not be instantiated. */
61111
private DqlUtils() {
62112
throw new AssertionError();

Diff for: projects/dctm-core/source/javatests/com/google/enterprise/connector/dctm/DctmMockAuthenticationManagerTest.java

+15
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,21 @@ public void testDomain_dnsParent() throws Exception {
369369
testParentDomainFail("ldapuser", "example.com");
370370
}
371371

372+
public void testSqlInjection_userNoDomain() throws Exception {
373+
testDomainFail("' or user_name = 'localuser", "");
374+
}
375+
376+
public void testSqlInjection_userDomain() throws Exception {
377+
testDomainFail(
378+
"ldapuser' and user_ldap_dn like '%dc=ajax,dc=example,dc=com,dc=au' --",
379+
"ajax.example.com");
380+
}
381+
382+
/** The post-processing using LdapName also blocks this. */
383+
public void testSqlInjection_domain() throws Exception {
384+
testDomainFail("ldapuser", "acme.example%");
385+
}
386+
372387
private Collection<String> toStrings(Collection<?> groups) {
373388
if (groups == null) {
374389
return null;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Copyright 2014 Google Inc. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.enterprise.connector.dctm;
16+
17+
import junit.framework.TestCase;
18+
19+
public class DqlUtilsTest extends TestCase {
20+
public void testEscapeString_null() {
21+
try {
22+
DqlUtils.escapeString(null);
23+
fail("Expected a NullPointerException");
24+
} catch (NullPointerException expected) {
25+
}
26+
}
27+
28+
public void testEscapeString_empty() {
29+
assertEquals("", DqlUtils.escapeString(""));
30+
}
31+
32+
public void testEscapeString_unescapedString() {
33+
String value = "hello world";
34+
assertEquals(value, DqlUtils.escapeString(value));
35+
}
36+
37+
public void testEscapeString_unescapedPattern() {
38+
String value = "\\ % _ \\\\ \\% \\_";
39+
assertEquals(value, DqlUtils.escapeString(value));
40+
}
41+
42+
public void testEscapeString_quoted() {
43+
String value = "' \\ % _ \\' \\\\ \\% \\_";
44+
assertEquals(value.replace("'", "''"), DqlUtils.escapeString(value));
45+
}
46+
47+
public void testEscapePattern_null() {
48+
try {
49+
DqlUtils.escapePattern(null, '\\');
50+
fail("Expected a NullPointerException");
51+
} catch (NullPointerException expected) {
52+
}
53+
}
54+
55+
public void testEscapePattern_empty() {
56+
assertEquals("", DqlUtils.escapePattern("", '\\'));
57+
}
58+
59+
public void testEscapePattern_unescapedString() {
60+
String value = "hello world";
61+
assertEquals(value, DqlUtils.escapePattern(value, '\\'));
62+
}
63+
64+
public void testEscapePattern_escapedPattern() {
65+
String value = "\\ % _ \\\\ \\% \\_";
66+
String escaped = "\\\\ \\% \\_ \\\\\\\\ \\\\\\% \\\\\\_";
67+
assertEquals(escaped, DqlUtils.escapePattern(value, '\\'));
68+
}
69+
70+
public void testEscapePattern_quoted() {
71+
String value = "' \\ % _ \\' \\\\ \\% \\_";
72+
String escaped = "'' \\\\ \\% \\_ \\\\'' \\\\\\\\ \\\\\\% \\\\\\_";
73+
assertEquals(escaped, DqlUtils.escapePattern(value, '\\'));
74+
}
75+
76+
public void testEscapePattern_quotedSlash() {
77+
String value = "' \\ % _ \\' \\\\ \\% \\_";
78+
String escaped = "'' /\\ /% /_ /\\'' /\\/\\ /\\/% /\\/_";
79+
assertEquals(escaped, DqlUtils.escapePattern(value, '/'));
80+
}
81+
}

0 commit comments

Comments
 (0)