Skip to content

Commit 83b3bbb

Browse files
committed
Resolved coderabbit review comments
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
1 parent b3b36e7 commit 83b3bbb

3 files changed

Lines changed: 23 additions & 3 deletions

File tree

registration/registration-services/src/main/java/io/mosip/registration/update/ResumableDownloader.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,10 @@ private ResumableDownloader() {
5757
* @param fileName the final file name within {@code targetDir}
5858
* @param connectTimeout connection timeout in milliseconds
5959
* @param readTimeout read timeout in milliseconds (0 = infinite)
60-
* @throws IOException if the download cannot be completed
60+
* @throws IOException if the download cannot be completed
61+
* @throws IllegalArgumentException if {@code fileName} is blank or contains a path separator or a
62+
* {@code ..} sequence (guards against path-traversal writes outside
63+
* {@code targetDir})
6164
*/
6265
public static void download(String url, String targetDir, String fileName,
6366
int connectTimeout, int readTimeout) throws IOException {
@@ -343,6 +346,12 @@ private static final class Artifact {
343346
private final File meta;
344347

345348
private Artifact(File dir, String name) {
349+
if (name == null || name.isEmpty()
350+
|| name.indexOf('/') >= 0 || name.indexOf('\\') >= 0
351+
|| name.contains("..")) {
352+
throw new IllegalArgumentException(
353+
"Illegal download file name (path traversal attempt?): " + name);
354+
}
346355
this.dir = dir;
347356
this.name = name;
348357
this.target = new File(dir, name);

registration/registration-services/src/main/java/io/mosip/registration/update/SoftwareUpdateUtil.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ protected static void downloadResumable(String url, String targetDir, String fil
129129
getTimeout(READ_TIMEOUT, DEFAULT_READ_TIMEOUT));
130130
} catch (IOException e) {
131131
LOGGER.error("Failed to download {}", url, e);
132-
throw new RegBaseCheckedException("REG-BUILD-005", "Failed to download " + url);
132+
throw new RegBaseCheckedException("REG-BUILD-005", "Failed to download " + url, e);
133133
}
134134
}
135135

@@ -146,7 +146,7 @@ protected static void ensureSpace(File targetDir, long requiredBytes) throws Reg
146146
try {
147147
ResumableDownloader.ensureSpace(targetDir, requiredBytes);
148148
} catch (IOException e) {
149-
throw new RegBaseCheckedException("REG-BUILD-004", e.getMessage());
149+
throw new RegBaseCheckedException("REG-BUILD-004", e.getMessage(), e);
150150
}
151151
}
152152

registration/registration-services/src/test/java/io/mosip/registration/test/update/SoftwareUpdateUtilTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,17 @@ public void downloadResumable_invalidHost_throws() throws Exception {
303303
downloadResumable("http://invalid.invalid/file", TEMP_DIR.getAbsolutePath(), "x.bin");
304304
}
305305

306+
@Test(expected = IllegalArgumentException.class)
307+
public void downloadResumable_pathTraversalName_rejected() throws Exception {
308+
// A malicious manifest entry must not be able to write outside the target directory.
309+
downloadResumable("http://localhost/file", TEMP_DIR.getAbsolutePath(), "..\\..\\evil.bin");
310+
}
311+
312+
@Test(expected = IllegalArgumentException.class)
313+
public void downloadResumable_separatorInName_rejected() throws Exception {
314+
downloadResumable("http://localhost/file", TEMP_DIR.getAbsolutePath(), "sub/evil.bin");
315+
}
316+
306317
@Test(expected = RegBaseCheckedException.class)
307318
public void downloadResumable_unexpectedStatus_throws() throws Exception {
308319
HttpServer server = HttpServer.create(new InetSocketAddress("localhost", 0), 0);

0 commit comments

Comments
 (0)