Skip to content

Commit dbb940b

Browse files
committed
chore: improve sanitization on concurreny queue logic
1 parent d468866 commit dbb940b

File tree

2 files changed

+36
-6
lines changed

2 files changed

+36
-6
lines changed

lib/core/Util.js

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,43 @@ export default function getUserAgent (sdk, application, integration, feature) {
104104
// URL validation functions to prevent SSRF attacks
105105
const isValidURL = (url) => {
106106
try {
107+
// Reject obviously malicious patterns early
108+
if (url.includes('@') || url.includes('file://') || url.includes('ftp://')) {
109+
return false
110+
}
111+
107112
// Allow relative URLs (they are safe as they use the same origin)
108113
if (url.startsWith('/') || url.startsWith('./') || url.startsWith('../')) {
109114
return true
110115
}
111116

112117
// Only validate absolute URLs for SSRF protection
113118
const parsedURL = new URL(url)
119+
120+
// Reject non-HTTP(S) protocols
121+
if (!['http:', 'https:'].includes(parsedURL.protocol)) {
122+
return false
123+
}
124+
125+
// Prevent IP addresses in URLs to avoid internal network access
126+
const ipv4Regex = /^(\d{1,3}\.){3}\d{1,3}$/
127+
const ipv6Regex = /^\[?([0-9a-fA-F]{0,4}:){2,7}[0-9a-fA-F]{0,4}\]?$/
128+
if (ipv4Regex.test(parsedURL.hostname) || ipv6Regex.test(parsedURL.hostname)) {
129+
// Only allow localhost IPs in development
130+
const isDevelopment = process.env.NODE_ENV === 'development' ||
131+
process.env.NODE_ENV === 'test' ||
132+
!process.env.NODE_ENV
133+
const localhostIPs = ['127.0.0.1', '0.0.0.0', '::1', 'localhost']
134+
if (!isDevelopment || !localhostIPs.includes(parsedURL.hostname)) {
135+
return false
136+
}
137+
}
138+
114139
return isAllowedHost(parsedURL.hostname)
115140
} catch (error) {
116141
// If URL parsing fails, it might be a relative URL without protocol
117-
// Allow it if it doesn't contain protocol indicators
118-
return !url.includes('://') && !url.includes('\\')
142+
// Allow it if it doesn't contain protocol indicators or suspicious patterns
143+
return !url.includes('://') && !url.includes('\\') && !url.includes('@')
119144
}
120145
}
121146

@@ -137,8 +162,12 @@ const isAllowedHost = (hostname) => {
137162
'0.0.0.0'
138163
]
139164

140-
// Allow localhost for development
141-
if (localhostPatterns.includes(hostname)) {
165+
// Only allow localhost in development environments to prevent SSRF in production
166+
const isDevelopment = process.env.NODE_ENV === 'development' ||
167+
process.env.NODE_ENV === 'test' ||
168+
!process.env.NODE_ENV // Default to allowing in non-production if NODE_ENV is not set
169+
170+
if (isDevelopment && localhostPatterns.includes(hostname)) {
142171
return true
143172
}
144173

lib/core/concurrency-queue.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,9 @@ export function ConcurrencyQueue ({ axios, config }) {
283283

284284
// Retry the requests that were pending due to token expiration
285285
this.running.forEach(({ request, resolve, reject }) => {
286-
// Retry the request
287-
axios(request).then(resolve).catch(reject)
286+
// Retry the request with sanitized configuration to prevent SSRF
287+
const sanitizedConfig = validateAndSanitizeConfig(request)
288+
axios(sanitizedConfig).then(resolve).catch(reject)
288289
})
289290
this.running = [] // Clear the running queue after retrying requests
290291
} catch (error) {

0 commit comments

Comments
 (0)