Fix security issues

nigoroll · Aug 24, 2011 · a58a452 · a58a452
1 parent d42a3a8
commit a58a452
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 10 deletions.
diff --git a/framework/src/play/CorePlugin.java b/framework/src/play/CorePlugin.java
@@ -66,6 +66,9 @@ public static String computeApplicationStatus(boolean json) {
     /**
      * Intercept /@status and check that the Authorization header is valid. 
      * Then ask each plugin for a status dump and send it over the HTTP response.
+     *
+     * You can ask the /@status using the authorization header and putting your status secret key in it.
+     * Prior to that you would be required to start play with  a -DstatusKey=yourkey
      */
     @Override
     public boolean rawInvocation(Request request, Response response) throws Exception {
@@ -81,7 +84,7 @@ public boolean rawInvocation(Request request, Response response) throws Exceptio
             }
             response.contentType = request.path.contains(".json") ? "application/json" : "text/plain";
             Header authorization = request.headers.get("authorization");
-            if (request.isLoopback || (authorization != null && Crypto.sign("@status").equals(authorization.value()))) {
+            if (authorization != null && (Crypto.sign("@status").equals(authorization.value()) || System.getProperty("statusKey", Play.secretKey).equals(authorization.value()))) {
                 response.print(computeApplicationStatus(request.path.contains(".json")));
                 response.status = 200;
                 return true;
@@ -139,12 +142,6 @@ public String getStatus() {
             out.println(plugin.index + ":" + plugin.getClass().getName());
         }
         out.println();
-        out.println("Configuration:");
-        out.println("~~~~~~~~~~~~~~");
-        for (Object key : Play.configuration.keySet()) {
-            out.println(key + "=" + Play.configuration.getProperty(key.toString()));
-        }
-        out.println();
         out.println("Threads:");
         out.println("~~~~~~~~");
         try {

diff --git a/framework/src/play/data/FileUpload.java b/framework/src/play/data/FileUpload.java
@@ -5,6 +5,9 @@
 import java.io.IOException;
 import java.io.InputStream;
 import org.apache.commons.fileupload.FileItem;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
+import play.Logger;
 import play.data.parsing.TempFilePlugin;
 import play.exceptions.UnexpectedException;
 import play.libs.Files;
@@ -21,9 +24,13 @@ public FileUpload() {
 
     public FileUpload(FileItem fileItem) {
         this.fileItem = fileItem;
-        defaultFile = new File(TempFilePlugin.createTempFolder(), fileItem.getFieldName() + File.separator + fileItem.getName());
-        defaultFile.getParentFile().mkdirs();
+        File tmp = TempFilePlugin.createTempFolder();
+        defaultFile = new File(tmp, FilenameUtils.getName(fileItem.getFieldName()) + File.separator + FilenameUtils.getName(fileItem.getName()));
         try {
+            if(!defaultFile.getCanonicalPath().startsWith(tmp.getCanonicalPath())) {
+                throw new IOException("Temp file try to override existing file?");
+            }
+            defaultFile.getParentFile().mkdirs();
             fileItem.write(defaultFile);
         } catch (Exception e) {
             throw new IllegalStateException("Error when trying to write to file " + defaultFile.getAbsolutePath(), e);

diff --git a/framework/src/play/data/parsing/ApacheMultipartParser.java b/framework/src/play/data/parsing/ApacheMultipartParser.java
@@ -27,6 +27,7 @@
 import org.apache.commons.fileupload.ParameterParser;
 import org.apache.commons.fileupload.disk.DiskFileItem;
 import org.apache.commons.io.FileCleaningTracker;
+import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.output.DeferredFileOutputStream;
 import play.Logger;
 import play.Play;
@@ -145,7 +146,7 @@ public AutoFileItem(FileItemStream stream) {
             this.fieldName = stream.getFieldName();
             this.contentType = stream.getContentType();
             this.isFormField = stream.isFormField();
-            this.fileName = stream.getName();
+            this.fileName = FilenameUtils.getName(stream.getName());
             this.sizeThreshold = Integer.parseInt(Play.configuration.getProperty("upload.threshold", "10240"));
             this.repository = null;
         }

diff --git a/framework/src/play/libs/Files.java b/framework/src/play/libs/Files.java
@@ -81,4 +81,5 @@ public static boolean copyDir(File from, File to) {
             return false;
         }
     }
+
 }