Skip to content

Commit 711eb01

Browse files
committed
Fix security issues
1 parent 43cbfde commit 711eb01

File tree

4 files changed

+19
-10
lines changed

4 files changed

+19
-10
lines changed

framework/src/play/CorePlugin.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ public static String computeApplicationStatus(boolean json) {
6969
/**
7070
* Intercept /@status and check that the Authorization header is valid.
7171
* Then ask each plugin for a status dump and send it over the HTTP response.
72+
*
73+
* You can ask the /@status using the authorization header and putting your status secret key in it.
74+
* Prior to that you would be required to start play with a -DstatusKey=yourkey
7275
*/
7376
@Override
7477
public boolean rawInvocation(Request request, Response response) throws Exception {
@@ -84,7 +87,7 @@ public boolean rawInvocation(Request request, Response response) throws Exceptio
8487
}
8588
response.contentType = request.path.contains(".json") ? "application/json" : "text/plain";
8689
Header authorization = request.headers.get("authorization");
87-
if (request.isLoopback || (authorization != null && Crypto.sign("@status").equals(authorization.value()))) {
90+
if (authorization != null && (Crypto.sign("@status").equals(authorization.value()) || System.getProperty("statusKey", Play.secretKey).equals(authorization.value()))) {
8891
response.print(computeApplicationStatus(request.path.contains(".json")));
8992
response.status = 200;
9093
return true;
@@ -142,12 +145,6 @@ public String getStatus() {
142145
out.println(plugin.index + ":" + plugin.getClass().getName() + " [" + (Play.pluginCollection.isEnabled(plugin) ? "enabled" : "disabled") + "]");
143146
}
144147
out.println();
145-
out.println("Configuration:");
146-
out.println("~~~~~~~~~~~~~~");
147-
for (Object key : Play.configuration.keySet()) {
148-
out.println(key + "=" + Play.configuration.getProperty(key.toString()));
149-
}
150-
out.println();
151148
out.println("Threads:");
152149
out.println("~~~~~~~~");
153150
try {

framework/src/play/data/FileUpload.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
import java.io.IOException;
66
import java.io.InputStream;
77
import org.apache.commons.fileupload.FileItem;
8+
import org.apache.commons.io.FileUtils;
9+
import org.apache.commons.io.FilenameUtils;
10+
import play.Logger;
811
import play.data.parsing.TempFilePlugin;
912
import play.exceptions.UnexpectedException;
1013
import play.libs.Files;
@@ -21,9 +24,13 @@ public FileUpload() {
2124

2225
public FileUpload(FileItem fileItem) {
2326
this.fileItem = fileItem;
24-
defaultFile = new File(TempFilePlugin.createTempFolder(), fileItem.getFieldName() + File.separator + fileItem.getName());
25-
defaultFile.getParentFile().mkdirs();
27+
File tmp = TempFilePlugin.createTempFolder();
28+
defaultFile = new File(tmp, FilenameUtils.getName(fileItem.getFieldName()) + File.separator + FilenameUtils.getName(fileItem.getName()));
2629
try {
30+
if(!defaultFile.getCanonicalPath().startsWith(tmp.getCanonicalPath())) {
31+
throw new IOException("Temp file try to override existing file?");
32+
}
33+
defaultFile.getParentFile().mkdirs();
2734
fileItem.write(defaultFile);
2835
} catch (Exception e) {
2936
throw new IllegalStateException("Error when trying to write to file " + defaultFile.getAbsolutePath(), e);

framework/src/play/data/parsing/ApacheMultipartParser.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import org.apache.commons.fileupload.disk.DiskFileItem;
2626
import org.apache.commons.io.FileCleaningTracker;
27+
import org.apache.commons.io.FilenameUtils;
2728
import org.apache.commons.io.output.DeferredFileOutputStream;
2829
import play.Logger;
2930
import play.Play;
@@ -142,7 +143,7 @@ public AutoFileItem(FileItemStream stream) {
142143
this.fieldName = stream.getFieldName();
143144
this.contentType = stream.getContentType();
144145
this.isFormField = stream.isFormField();
145-
this.fileName = stream.getName();
146+
this.fileName = FilenameUtils.getName(stream.getName());
146147
this.sizeThreshold = Integer.parseInt(Play.configuration.getProperty("upload.threshold", "10240"));
147148
this.repository = null;
148149
}

framework/src/play/libs/Files.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ public static boolean copyDir(File from, File to) {
8989

9090
public static void unzip(File from, File to) {
9191
try {
92+
String outDir = to.getCanonicalPath();
9293
ZipFile zipFile = new ZipFile(from);
9394
Enumeration<? extends ZipEntry> entries = zipFile.entries();
9495
while (entries.hasMoreElements()) {
@@ -98,6 +99,9 @@ public static void unzip(File from, File to) {
9899
continue;
99100
}
100101
File f = new File(to, entry.getName());
102+
if(!f.getCanonicalPath().startsWith(outDir)) {
103+
throw new IOException("Corrupted zip file");
104+
}
101105
f.getParentFile().mkdirs();
102106
FileOutputStream os = new FileOutputStream(f);
103107
IO.copy(zipFile.getInputStream(entry), os);

0 commit comments

Comments
 (0)