Skip to content

Commit 5fb0a8a

Browse files
sfc-gh-dheymansfc-gh-dheyman
authored
SNOW-3104251: Add sanitization for nonProxyHost RegEx patterns (#2506)
1 parent 8901889 commit 5fb0a8a

File tree

8 files changed

+133
-26
lines changed

8 files changed

+133
-26
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
- Fix expired session token renewal when polling results (snowflakedb/snowflake-jdbc#2489)
66
- Fix missing minicore async initialization that was dropped during public API restructuring in v4.0.0
77
- Adjust level of logging during Driver initialization
8+
- Add sanitization for nonProxyHosts RegEx patterns
89

910
- v4.0.1
1011
- Add /etc/os-release data to Minicore telemetry

‎src/main/java/net/snowflake/client/internal/core/SdkProxyRoutePlanner.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,8 @@ public SdkProxyRoutePlanner(
2222
String proxyHost, int proxyPort, HttpProtocol proxyProtocol, String nonProxyHosts) {
2323
super(DefaultSchemePortResolver.INSTANCE);
2424
proxy = new HttpHost(proxyHost, proxyPort, proxyProtocol.toString());
25-
// parseNonProxyHosts
2625
if (!SnowflakeUtil.isNullOrEmpty(nonProxyHosts)) {
27-
String[] hosts = nonProxyHosts.split("\\|");
28-
hostPatterns = new String[hosts.length];
29-
for (int i = 0; i < hosts.length; ++i) {
30-
hostPatterns[i] = hosts[i].toLowerCase().replace("*", ".*?");
31-
}
26+
hostPatterns = nonProxyHosts.split("\\|");
3227
} else {
3328
hostPatterns = null;
3429
}
@@ -39,7 +34,8 @@ private boolean doesTargetMatchNonProxyHosts(HttpHost target) {
3934
return false;
4035
}
4136
String targetHost = target.getHostName().toLowerCase();
42-
return Arrays.stream(hostPatterns).anyMatch(targetHost::matches);
37+
return Arrays.stream(hostPatterns)
38+
.anyMatch(pattern -> SnowflakeUtil.hostnameMatchesGlob(targetHost, pattern));
4339
}
4440

4541
@Override

‎src/main/java/net/snowflake/client/internal/jdbc/SnowflakeUtil.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.util.concurrent.ThreadFactory;
4242
import java.util.concurrent.ThreadPoolExecutor;
4343
import java.util.concurrent.TimeUnit;
44+
import java.util.regex.Pattern;
4445
import java.util.stream.Collectors;
4546
import net.snowflake.client.api.exception.ErrorCode;
4647
import net.snowflake.client.api.exception.SnowflakeSQLException;
@@ -1006,4 +1007,22 @@ public static String byteToHexString(byte[] bytes) {
10061007
}
10071008
return new String(hexChars);
10081009
}
1010+
1011+
/**
1012+
* Converts a simple wildcard pattern (where only '*' is special) into a safe regex string by
1013+
* quoting all literal parts via Pattern.quote(), preventing any regex metacharacters from being
1014+
* interpreted. This avoids ReDoS vulnerabilities when patterns originate from user input.
1015+
*/
1016+
public static Pattern globToSafePattern(String glob) {
1017+
String safeRegex =
1018+
Arrays.stream(glob.split("\\*", -1)).map(Pattern::quote).collect(Collectors.joining(".*"));
1019+
return Pattern.compile(safeRegex, Pattern.CASE_INSENSITIVE);
1020+
}
1021+
1022+
public static boolean hostnameMatchesGlob(String hostname, String pattern) {
1023+
if (hostname == null || pattern == null) {
1024+
return false;
1025+
}
1026+
return globToSafePattern(pattern.trim()).matcher(hostname).matches();
1027+
}
10091028
}

‎src/main/java/net/snowflake/client/internal/jdbc/cloud/storage/S3HttpUtil.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import net.snowflake.client.internal.core.HttpClientSettingsKey;
1212
import net.snowflake.client.internal.core.HttpUtil;
1313
import net.snowflake.client.internal.core.SFSessionProperty;
14+
import net.snowflake.client.internal.jdbc.SnowflakeUtil;
1415
import net.snowflake.client.internal.log.SFLogger;
1516
import net.snowflake.client.internal.log.SFLoggerFactory;
1617
import net.snowflake.client.internal.log.SFLoggerUtil;
@@ -150,15 +151,7 @@ public static ProxyConfiguration createSessionlessProxyConfigurationForS3(
150151
static Set<String> prepareNonProxyHosts(String nonProxyHosts) {
151152
return Arrays.stream(nonProxyHosts.split("\\|"))
152153
.map(String::trim)
153-
.map(
154-
host -> {
155-
// AWS SDK v2 Netty client expects proper Java regex patterns for non-proxy hosts.
156-
// Transform traditional proxy patterns to valid regex:
157-
// 1. Escape dots to match literal periods
158-
// 2. Replace wildcards (*) with regex equivalent (.*)
159-
// This differs from SdkProxyRoutePlanner which uses simple pattern matching
160-
return host.replace(".", "\\.").replace("*", ".*?");
161-
})
154+
.map(host -> SnowflakeUtil.globToSafePattern(host).pattern())
162155
.collect(Collectors.toSet());
163156
}
164157
}

‎src/main/java/net/snowflake/client/internal/jdbc/diagnostic/ProxyConfig.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import java.net.InetSocketAddress;
44
import java.net.Proxy;
5-
import java.util.regex.Pattern;
5+
import net.snowflake.client.internal.jdbc.SnowflakeUtil;
66
import net.snowflake.client.internal.log.SFLogger;
77
import net.snowflake.client.internal.log.SFLoggerFactory;
88

@@ -172,10 +172,13 @@ private void resolveProxyConfigurations() {
172172
}
173173

174174
protected boolean isBypassProxy(String hostname) {
175-
String nonProxyHosts = getNonProxyHosts().replace(".", "\\.").replace("*", ".*");
175+
String nonProxyHosts = getNonProxyHosts();
176+
if (nonProxyHosts == null || nonProxyHosts.isEmpty()) {
177+
return false;
178+
}
176179
String[] nonProxyHostsArray = nonProxyHosts.split("\\|");
177-
for (String i : nonProxyHostsArray) {
178-
if (Pattern.compile(i).matcher(hostname).matches()) {
180+
for (String pattern : nonProxyHostsArray) {
181+
if (SnowflakeUtil.hostnameMatchesGlob(hostname, pattern)) {
179182
return true;
180183
}
181184
}

‎src/test/java/net/snowflake/client/internal/core/CoreUtilsMiscellaneousTest.java‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static net.snowflake.client.internal.jdbc.SnowflakeUtil.systemGetProperty;
44
import static org.junit.jupiter.api.Assertions.assertEquals;
55
import static org.junit.jupiter.api.Assertions.assertFalse;
6+
import static org.junit.jupiter.api.Assertions.assertNotNull;
67
import static org.junit.jupiter.api.Assertions.assertNull;
78
import static org.junit.jupiter.api.Assertions.assertThrows;
89
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -19,6 +20,7 @@
1920
import net.snowflake.client.api.exception.SnowflakeSQLException;
2021
import net.snowflake.client.internal.jdbc.SnowflakeUtil;
2122
import net.snowflake.client.internal.jdbc.cloud.storage.S3HttpUtil;
23+
import org.apache.http.HttpHost;
2224
import org.junit.jupiter.api.Test;
2325
import software.amazon.awssdk.http.nio.netty.ProxyConfiguration;
2426

@@ -114,7 +116,7 @@ public void testSetProxyForS3() {
114116
assertEquals("snowflakecomputing.com", proxyConfiguration.host());
115117
assertEquals(443, proxyConfiguration.port());
116118
assertEquals(
117-
new HashSet<>(Collections.singletonList(".*?\\.foo\\.com")),
119+
new HashSet<>(Collections.singletonList("\\Q\\E.*\\Q.foo.com\\E")),
118120
proxyConfiguration.nonProxyHosts());
119121
assertEquals("pw", proxyConfiguration.password());
120122
assertEquals("testuser", proxyConfiguration.username());
@@ -136,7 +138,8 @@ public void testSetSessionlessProxyForS3() throws SnowflakeSQLException {
136138
assertEquals("localhost", proxyConfiguration.host());
137139
assertEquals(8084, proxyConfiguration.port());
138140
assertEquals(
139-
new HashSet<>(Arrays.asList("baz\\.com", "foo\\.com")), proxyConfiguration.nonProxyHosts());
141+
new HashSet<>(Arrays.asList("\\Qbaz.com\\E", "\\Qfoo.com\\E")),
142+
proxyConfiguration.nonProxyHosts());
140143
assertEquals("pw", proxyConfiguration.password());
141144
assertEquals("testuser", proxyConfiguration.username());
142145
// Test that exception is thrown when port number is invalid
@@ -290,6 +293,38 @@ public void testSizeOfHttpClientMapWithGzipAndUserAgentSuffix() {
290293
assertEquals(3, HttpUtil.httpClient.size());
291294
}
292295

296+
@Test
297+
public void testSdkProxyRoutePlannerNonProxyHostsBypassesProxy() throws Exception {
298+
SdkProxyRoutePlanner planner =
299+
new SdkProxyRoutePlanner(
300+
"proxy.example.com", 8080, HttpProtocol.HTTP, "*.internal.com|localhost");
301+
// Hosts matching nonProxyHosts should bypass proxy (determineProxy returns null)
302+
HttpHost internalHost = new HttpHost("app.internal.com");
303+
HttpHost localhostHost = new HttpHost("localhost");
304+
HttpHost externalHost = new HttpHost("external.com");
305+
306+
assertNull(planner.determineProxy(internalHost, null, null));
307+
assertNull(planner.determineProxy(localhostHost, null, null));
308+
assertNotNull(planner.determineProxy(externalHost, null, null));
309+
}
310+
311+
@Test
312+
public void testSdkProxyRoutePlannerReDoSPatternDoesNotHang() throws Exception {
313+
// The exact ReDoS payload from the vulnerability report
314+
SdkProxyRoutePlanner planner =
315+
new SdkProxyRoutePlanner("proxy.example.com", 8080, HttpProtocol.HTTP, "(a+)+");
316+
String maliciousHost =
317+
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaac";
318+
HttpHost target = new HttpHost(maliciousHost);
319+
320+
long start = System.currentTimeMillis();
321+
assertNotNull(planner.determineProxy(target, null, null));
322+
long elapsed = System.currentTimeMillis() - start;
323+
assertTrue(
324+
elapsed < 1000,
325+
"Non-proxy host matching should complete nearly instantly, took " + elapsed + "ms");
326+
}
327+
293328
@Test
294329
public void testConvertProxyPropertiesToHttpClientKey() throws SnowflakeSQLException {
295330
OCSPMode mode = OCSPMode.FAIL_OPEN;

‎src/test/java/net/snowflake/client/internal/jdbc/SnowflakeUtilTest.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static net.snowflake.client.internal.jdbc.SnowflakeUtil.createOwnerOnlyPermissionDir;
55
import static net.snowflake.client.internal.jdbc.SnowflakeUtil.extractColumnMetadata;
66
import static net.snowflake.client.internal.jdbc.SnowflakeUtil.getSnowflakeType;
7+
import static net.snowflake.client.internal.jdbc.SnowflakeUtil.hostnameMatchesGlob;
78
import static org.junit.jupiter.api.Assertions.assertEquals;
89
import static org.junit.jupiter.api.Assertions.assertFalse;
910
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -239,6 +240,62 @@ private static ObjectNode createFieldNode(
239240
return fieldNode;
240241
}
241242

243+
@Test
244+
public void testHostnameMatchesGlob() {
245+
// Exact match
246+
assertTrue(hostnameMatchesGlob("localhost", "localhost"));
247+
assertTrue(hostnameMatchesGlob("example.com", "example.com"));
248+
assertFalse(hostnameMatchesGlob("example.com", "other.com"));
249+
250+
// Case insensitive
251+
assertTrue(hostnameMatchesGlob("EXAMPLE.COM", "example.com"));
252+
assertTrue(hostnameMatchesGlob("example.com", "EXAMPLE.COM"));
253+
254+
// Leading wildcard
255+
assertTrue(hostnameMatchesGlob("foo.example.com", "*.example.com"));
256+
assertTrue(hostnameMatchesGlob("bar.baz.example.com", "*.example.com"));
257+
assertFalse(hostnameMatchesGlob("example.com", "*.example.com"));
258+
assertFalse(hostnameMatchesGlob("notexample.com", "*.example.com"));
259+
260+
// Trailing wildcard
261+
assertTrue(hostnameMatchesGlob("192.168.1.1", "192.168.*"));
262+
assertFalse(hostnameMatchesGlob("10.0.0.1", "192.168.*"));
263+
264+
// Middle wildcard
265+
assertTrue(hostnameMatchesGlob("foo.bar.com", "foo.*.com"));
266+
assertFalse(hostnameMatchesGlob("bar.baz.com", "foo.*.com"));
267+
268+
// Wildcard only
269+
assertTrue(hostnameMatchesGlob("anything.example.com", "*"));
270+
assertTrue(hostnameMatchesGlob("", "*"));
271+
272+
// Whitespace trimming
273+
assertTrue(hostnameMatchesGlob("example.com", " example.com "));
274+
275+
// Null safety
276+
assertFalse(hostnameMatchesGlob(null, "*.example.com"));
277+
assertFalse(hostnameMatchesGlob("example.com", null));
278+
assertFalse(hostnameMatchesGlob(null, null));
279+
280+
// Regex metacharacters are treated as literals, not regex
281+
assertFalse(hostnameMatchesGlob("aaa", "(a+)+"));
282+
assertFalse(hostnameMatchesGlob("exampleXcom", "example.com"));
283+
assertTrue(hostnameMatchesGlob("example.com", "example.com"));
284+
}
285+
286+
@Test
287+
public void testHostnameMatchesGlobReDoSPatternDoesNotHang() {
288+
// Exact ReDoS payload from the vulnerability report (SNOW-3104251).
289+
// With unquoted regex this would cause catastrophic backtracking.
290+
String maliciousHost =
291+
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaac";
292+
long start = System.currentTimeMillis();
293+
assertFalse(hostnameMatchesGlob(maliciousHost, "(a+)+"));
294+
long elapsed = System.currentTimeMillis() - start;
295+
assertTrue(
296+
elapsed < 1000, "Glob matching should complete nearly instantly, took " + elapsed + "ms");
297+
}
298+
242299
@Test
243300
public void testFieldMetadataSetterMethods() {
244301
FieldMetadataImpl fieldMetadata = new FieldMetadataImpl();

‎src/test/java/net/snowflake/client/internal/jdbc/cloud/storage/S3HttpUtilTest.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,19 @@
1313
class S3HttpUtilTest {
1414
static Stream<Arguments> prepareNonProxyHostsTestCases() {
1515
return Stream.of(
16-
Arguments.of("example.com", new HashSet<>(Arrays.asList("example\\.com"))),
16+
Arguments.of("example.com", new HashSet<>(Arrays.asList("\\Qexample.com\\E"))),
1717
Arguments.of(
1818
"example.com|test.org | localhost",
19-
new HashSet<>(Arrays.asList("example\\.com", "test\\.org", "localhost"))),
20-
Arguments.of("*.example.com", new HashSet<>(Arrays.asList(".*?\\.example\\.com"))),
19+
new HashSet<>(Arrays.asList("\\Qexample.com\\E", "\\Qtest.org\\E", "\\Qlocalhost\\E"))),
20+
Arguments.of("*.example.com", new HashSet<>(Arrays.asList("\\Q\\E.*\\Q.example.com\\E"))),
2121
Arguments.of(
2222
"example.com|*.test.org|localhost|*.internal.*",
2323
new HashSet<>(
2424
Arrays.asList(
25-
"example\\.com", ".*?\\.test\\.org", "localhost", ".*?\\.internal\\..*?"))));
25+
"\\Qexample.com\\E",
26+
"\\Q\\E.*\\Q.test.org\\E",
27+
"\\Qlocalhost\\E",
28+
"\\Q\\E.*\\Q.internal.\\E.*\\Q\\E"))));
2629
}
2730

2831
@ParameterizedTest

0 commit comments

Comments
 (0)