Skip to content

Commit 46a6bba

Browse files
DaanHooglandnvazquez
authored andcommitted
Fix: KVM Direct Download URL injection (apache#60)
Co-authored-by: nvazquez <nicovazquez90@gmail.com>
1 parent 1f849ca commit 46a6bba

3 files changed

Lines changed: 8 additions & 7 deletions

File tree

core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.cloud.utils.UriUtils;
2222
import com.cloud.utils.exception.CloudRuntimeException;
2323
import org.apache.cloudstack.utils.security.DigestHelper;
24+
import org.apache.commons.io.FilenameUtils;
2425
import org.apache.commons.lang3.StringUtils;
2526
import org.apache.logging.log4j.Logger;
2627
import org.apache.logging.log4j.LogManager;
@@ -33,6 +34,7 @@
3334
import java.util.ArrayList;
3435
import java.util.List;
3536
import java.util.Map;
37+
import java.util.UUID;
3638

3739
public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDownloader {
3840

@@ -128,15 +130,14 @@ public void setFollowRedirects(boolean followRedirects) {
128130
*/
129131
protected File createTemporaryDirectoryAndFile(String downloadDir) {
130132
createFolder(downloadDir);
131-
return new File(downloadDir + File.separator + getFileNameFromUrl());
133+
return new File(downloadDir + File.separator + getTemporaryFileName());
132134
}
133135

134136
/**
135-
* Return filename from url
137+
* Return filename from the temporary download file
136138
*/
137-
public String getFileNameFromUrl() {
138-
String[] urlParts = url.split("/");
139-
return urlParts[urlParts.length - 1];
139+
public String getTemporaryFileName() {
140+
return String.format("%s.%s", UUID.randomUUID(), FilenameUtils.getExtension(url));
140141
}
141142

142143
@Override

core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public Pair<Boolean, String> downloadTemplate() {
9797
DirectTemplateDownloader urlDownloader = createDownloaderForMetalinks(getUrl(), getTemplateId(), getDestPoolPath(),
9898
getChecksum(), headers, connectTimeout, soTimeout, null, temporaryDownloadPath);
9999
try {
100-
setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl());
100+
setDownloadedFilePath(downloadDir + File.separator + getTemporaryFileName());
101101
File f = new File(getDownloadedFilePath());
102102
if (f.exists()) {
103103
f.delete();

core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public Pair<Boolean, String> downloadTemplate() {
6969
String mount = String.format(mountCommand, srcHost + ":" + srcPath, "/mnt/" + mountSrcUuid);
7070
Script.runSimpleBashScript(mount);
7171
String downloadDir = getDestPoolPath() + File.separator + getDirectDownloadTempPath(getTemplateId());
72-
setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl());
72+
setDownloadedFilePath(downloadDir + File.separator + getTemporaryFileName());
7373
Script.runSimpleBashScript("cp /mnt/" + mountSrcUuid + srcPath + " " + getDownloadedFilePath());
7474
Script.runSimpleBashScript("umount /mnt/" + mountSrcUuid);
7575
return new Pair<>(true, getDownloadedFilePath());

0 commit comments

Comments
 (0)