Skip to content

Commit 444190b

Browse files
D-D-HCopilotCopilot
authored
Restrict file transfer URL scheme to http(s) (#392)
Signed-off-by: Denghui Dong <denghui.ddh@alibaba-inc.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 20b1a25 commit 444190b

3 files changed

Lines changed: 82 additions & 2 deletions

File tree

server/src/main/java/org/eclipse/jifa/server/domain/dto/FileTransferRequest.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public boolean isValid(FileTransferRequest request, ConstraintValidatorContext c
197197
valid &= checkNotBlank(request.scpSourcePath, "scpSourcePath", context);
198198
}
199199

200-
case URL -> valid &= checkNotBlank(request.url, "url", context);
200+
case URL -> valid &= checkHttpUrl(request.url, "url", context);
201201

202202
case TEXT -> {
203203
// TODO: should check file type
@@ -222,6 +222,35 @@ private boolean checkNotBlank(String value, String name, ConstraintValidatorCont
222222
return true;
223223
}
224224

225+
/**
226+
* Ensures the value is a non-blank http(s) URL. Other schemes such as file, jar and ftp
227+
* are rejected to prevent reading arbitrary local resources on the server host.
228+
*/
229+
private boolean checkHttpUrl(String value, String name, ConstraintValidatorContext context) {
230+
if (!checkNotBlank(value, name, context)) {
231+
return false;
232+
}
233+
234+
java.net.URL url;
235+
try {
236+
url = new java.net.URL(value);
237+
} catch (java.net.MalformedURLException e) {
238+
context.buildConstraintViolationWithTemplate("Only http and https URLs are supported")
239+
.addPropertyNode(name)
240+
.addConstraintViolation();
241+
return false;
242+
}
243+
244+
String protocol = url.getProtocol();
245+
if (!("http".equalsIgnoreCase(protocol) || "https".equalsIgnoreCase(protocol)) || StringUtils.isBlank(url.getHost())) {
246+
context.buildConstraintViolationWithTemplate("Only http and https URLs are supported")
247+
.addPropertyNode(name)
248+
.addConstraintViolation();
249+
return false;
250+
}
251+
return true;
252+
}
253+
225254
private boolean checkTrue(String value, String name, ConstraintValidatorContext context) {
226255
if (StringUtils.isBlank(value)) {
227256
context.buildConstraintViolationWithTemplate("{jakarta.validation.constraints.NotBlank.message}")

server/src/main/java/org/eclipse/jifa/server/service/impl/StorageServiceImpl.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,14 @@ public StreamCopier.Listener file(String name, long size) {
321321
}
322322

323323
private void transferByURL(FileTransferRequest request, Path destination, FileTransferListener listener) throws IOException {
324-
URLConnection conn = new java.net.URL(request.getUrl()).openConnection();
324+
java.net.URL url = new java.net.URL(request.getUrl());
325+
String protocol = url.getProtocol();
326+
// Only http(s) is allowed. Reject schemes such as file, jar, ftp, etc. to
327+
// prevent reading arbitrary local files (or other resources) on the host.
328+
Validate.isTrue(("http".equalsIgnoreCase(protocol) || "https".equalsIgnoreCase(protocol))
329+
&& StringUtils.isNotBlank(url.getHost()),
330+
CommonErrorCode.ILLEGAL_ARGUMENT, "Only http and https URLs are supported");
331+
URLConnection conn = url.openConnection();
325332
listener.fireTotalSize(Math.max(conn.getContentLengthLong(), 0));
326333
try (InputStream in = conn.getInputStream();
327334
OutputStream out = new FileOutputStream(destination.toFile())) {

server/src/test/java/org/eclipse/jifa/server/domain/dto/TestFileTransferRequest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,55 @@ public void test() {
124124
result = validator.validate(request);
125125
shouldBeIllegal(result, "url");
126126

127+
// Non http(s) schemes must be rejected to prevent local file disclosure (SSRF).
128+
request = new FileTransferRequest();
129+
request.setType(FileType.GC_LOG);
130+
request.setMethod(FileTransferMethod.URL);
131+
request.setUrl("file:///etc/passwd");
132+
result = validator.validate(request);
133+
shouldBeIllegal(result, "url");
134+
135+
request = new FileTransferRequest();
136+
request.setType(FileType.GC_LOG);
137+
request.setMethod(FileTransferMethod.URL);
138+
request.setUrl("file://localhost/etc/passwd");
139+
result = validator.validate(request);
140+
shouldBeIllegal(result, "url");
141+
142+
request = new FileTransferRequest();
143+
request.setType(FileType.GC_LOG);
144+
request.setMethod(FileTransferMethod.URL);
145+
request.setUrl("jar:file:/tmp/a.jar!/b");
146+
result = validator.validate(request);
147+
shouldBeIllegal(result, "url");
148+
149+
request = new FileTransferRequest();
150+
request.setType(FileType.GC_LOG);
151+
request.setMethod(FileTransferMethod.URL);
152+
request.setUrl("ftp://example.org/data.txt");
153+
result = validator.validate(request);
154+
shouldBeIllegal(result, "url");
155+
156+
// A URL without a scheme is not a valid http(s) URL.
127157
request = new FileTransferRequest();
128158
request.setType(FileType.GC_LOG);
129159
request.setMethod(FileTransferMethod.URL);
130160
request.setUrl("url");
131161
result = validator.validate(request);
162+
shouldBeIllegal(result, "url");
163+
164+
request = new FileTransferRequest();
165+
request.setType(FileType.GC_LOG);
166+
request.setMethod(FileTransferMethod.URL);
167+
request.setUrl("http://example.org/data.txt");
168+
result = validator.validate(request);
169+
assertEquals(0, result.size());
170+
171+
request = new FileTransferRequest();
172+
request.setType(FileType.GC_LOG);
173+
request.setMethod(FileTransferMethod.URL);
174+
request.setUrl("https://example.org/data.txt");
175+
result = validator.validate(request);
132176
assertEquals(0, result.size());
133177
}
134178

0 commit comments

Comments
 (0)