CVE-2009-2693

@@ -28,8 +28,10 @@ standardLoader.reloading=Reloading checks are enabled for this Context
 standardLoader.removeRepository=Removing repository {0}
 standardLoader.starting=Starting this Loader
 standardLoader.stopping=Stopping this Loader
+webappClassLoader.illegalJarPath=Illegal JAR entry detected with name {0}
 webappClassLoader.stopped=Illegal access: this web application instance has been stopped already.  Could not load {0}.  The eventual following stack trace is caused by an error thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access, and has no functional impact.
 webappClassLoader.readError=Resource read error: Could not load {0}.
+webappClassLoader.validationErrorJarPath=Unable to validate JAR entry with name {0}
 webappClassLoader.wrongVersion=(unable to load class {0})
 webappLoader.addRepository=Adding repository {0}
 webappLoader.deploy=Deploying class repositories to work directory {0}

@@ -358,6 +358,7 @@ public WebappClassLoader(ClassLoader parent) {
      * Path where resources loaded from JARs will be extracted.
      */
     protected File loaderDir = null;
+    protected String canonicalLoaderDir = null;
 
 
     /**
@@ -551,6 +552,19 @@ public void setJarPath(String jarPath) {
      */
     public void setWorkDir(File workDir) {
         this.loaderDir = new File(workDir, "loader");
+        if (loaderDir == null) {
+            canonicalLoaderDir = null;
+        } else { 
+            try {
+                canonicalLoaderDir = loaderDir.getCanonicalPath();
+                if (!canonicalLoaderDir.endsWith(File.separator)) {
+                    canonicalLoaderDir += File.separator;
+                }
+            } catch (IOException ioe) {
+                canonicalLoaderDir = null;
+            }
+        }
+
     }
 
      /**
@@ -2118,6 +2132,18 @@ protected ResourceEntry findResourceInternal(String name, String path) {
                                         (".class"))) {
                                     resourceFile = new File
                                         (loaderDir, jarEntry2.getName());
+                                    try {
+                                        if (!resourceFile.getCanonicalPath().startsWith(
+                                                canonicalLoaderDir)) {
+                                            throw new IllegalArgumentException(
+                                                    sm.getString("webappClassLoader.illegalJarPath",
+                                                            jarEntry2.getName()));
+                                        }
+                                    } catch (IOException ioe) {
+                                        throw new IllegalArgumentException(
+                                                sm.getString("webappClassLoader.validationErrorJarPath",
+                                                        jarEntry2.getName()), ioe);
+                                    }
                                     resourceFile.getParentFile().mkdirs();
                                     FileOutputStream os = null;
                                     InputStream is = null;

@@ -852,35 +852,40 @@ protected void fixDocBase()
         file = new File(docBase);
         String origDocBase = docBase ;
  
-        String contextPath = context.getPath();
-        if (contextPath.equals("")) {
-            contextPath = "ROOT";
+        String pathName = context.getPath();
+        if (pathName.equals("")) {
+            pathName = "ROOT";
         } else {
-            if (contextPath.lastIndexOf('/') > 0) {
-                contextPath = "/" + contextPath.substring(1).replace('/','#');
-            }
+            // Context path must start with '/'
+            pathName = pathName.substring(1).replace('/','#');
         }
         if (docBase.toLowerCase().endsWith(".war") && !file.isDirectory() && unpackWARs) {
             URL war = new URL("jar:" + (new File(docBase)).toURI().toURL() + "!/");
-            docBase = ExpandWar.expand(host, war, contextPath);
+            docBase = ExpandWar.expand(host, war, pathName);
             file = new File(docBase);
             docBase = file.getCanonicalPath();
             if (context instanceof StandardContext) {
                 ((StandardContext) context).setOriginalDocBase(origDocBase);
             }
+        } else if (docBase.toLowerCase().endsWith(".war") &&
+                !file.isDirectory() && !unpackWARs) {
+            URL war =
+                new URL("jar:" + (new File (docBase)).toURI().toURL() + "!/");
+            ExpandWar.validate(host, war, pathName);
         } else {
             File docDir = new File(docBase);
             if (!docDir.exists()) {
                 File warFile = new File(docBase + ".war");
                 if (warFile.exists()) {
+                    URL war =
+                        new URL("jar:" + warFile.toURI().toURL() + "!/");
                     if (unpackWARs) {
-                        URL war =
-                            new URL("jar:" + warFile.toURI().toURL() + "!/");
-                        docBase = ExpandWar.expand(host, war, contextPath);
+                        docBase = ExpandWar.expand(host, war, pathName);
                         file = new File(docBase);
                         docBase = file.getCanonicalPath();
                     } else {
                         docBase = warFile.getCanonicalPath();
+                        ExpandWar.validate(host, war, pathName);
                     }
                 }
                 if (context instanceof StandardContext) {
@@ -1238,7 +1243,8 @@ protected synchronized void stop() {
             if (!docBaseFile.isAbsolute()) {
                 docBaseFile = new File(appBase, docBase);
             }
-            ExpandWar.delete(docBaseFile);
+            // No need to log failure - it is expected in this case
+            ExpandWar.delete(docBaseFile, false);
         }
         
         ok = true;

@@ -105,7 +105,8 @@ public static String expand(Host host, URL war)
      *  (must start with "jar:")
      * @param pathname Context path name for web application
      *
-     * @exception IllegalArgumentException if this is not a "jar:" URL
+     * @exception IllegalArgumentException if this is not a "jar:" URL or if the
+     *            WAR file is invalid
      * @exception IOException if an input/output error was encountered
      *  during expansion
      */
@@ -123,6 +124,7 @@ public static String expand(Host host, URL war, String pathname)
                 (sm.getString("hostConfig.appBase",
                               appBase.getAbsolutePath()));
         }
+        
         File docBase = new File(appBase, pathname);
         if (docBase.exists()) {
             // War file is already installed
@@ -133,16 +135,29 @@ public static String expand(Host host, URL war, String pathname)
         docBase.mkdir();
 
         // Expand the WAR into the new document base directory
+        String canonicalDocBasePrefix = docBase.getCanonicalPath();
+        if (!canonicalDocBasePrefix.endsWith(File.separator)) {
+            canonicalDocBasePrefix += File.separator;
+        }
         JarURLConnection juc = (JarURLConnection) war.openConnection();
         juc.setUseCaches(false);
         JarFile jarFile = null;
         InputStream input = null;
+        boolean success = false;
         try {
             jarFile = juc.getJarFile();
             Enumeration jarEntries = jarFile.entries();
             while (jarEntries.hasMoreElements()) {
                 JarEntry jarEntry = (JarEntry) jarEntries.nextElement();
                 String name = jarEntry.getName();
+                File expandedFile = new File(docBase, name);
+                if (!expandedFile.getCanonicalPath().startsWith(
+                        canonicalDocBasePrefix)) {
+                    // Trying to expand outside the docBase
+                    // Throw an exception to stop the deployment
+                    throw new IllegalArgumentException(
+                            sm.getString("expandWar.illegalPath",war, name));
+                }
                 int last = name.lastIndexOf('/');
                 if (last >= 0) {
                     File parent = new File(docBase,
@@ -155,21 +170,24 @@ public static String expand(Host host, URL war, String pathname)
                 input = jarFile.getInputStream(jarEntry);
 
                 // Bugzilla 33636
-                File expandedFile = expand(input, docBase, name);
+                expand(input, expandedFile);
                 long lastModified = jarEntry.getTime();
-                if ((lastModified != -1) && (lastModified != 0) && (expandedFile != null)) {
+                if ((lastModified != -1) && (lastModified != 0)) {
                     expandedFile.setLastModified(lastModified);
                 }
 
                 input.close();
                 input = null;
             }
+            success = true;
         } catch (IOException e) {
-            // If something went wrong, delete expanded dir to keep things 
-            // clean
-            deleteDir(docBase);
             throw e;
         } finally {
+            if (!success) {
+                // If something went wrong, delete expanded dir to keep things 
+                // clean
+                deleteDir(docBase);
+            }
             if (input != null) {
                 try {
                     input.close();
@@ -194,6 +212,69 @@ public static String expand(Host host, URL war, String pathname)
     }
 
 
+    /**
+     * Validate the WAR file found at the specified URL.
+     *
+     * @param host Host war is being installed for
+     * @param war URL of the web application archive to be validated
+     *  (must start with "jar:")
+     * @param pathname Context path name for web application
+     *
+     * @exception IllegalArgumentException if this is not a "jar:" URL or if the
+     *            WAR file is invalid
+     * @exception IOException if an input/output error was encountered
+     *            during validation
+     */
+    public static void validate(Host host, URL war, String pathname)
+        throws IOException {
+
+        // Make the appBase absolute
+        File appBase = new File(host.getAppBase());
+        if (!appBase.isAbsolute()) {
+            appBase = new File(System.getProperty("catalina.base"),
+                               host.getAppBase());
+        }
+        
+        File docBase = new File(appBase, pathname);
+
+        // Calculate the document base directory
+        String canonicalDocBasePrefix = docBase.getCanonicalPath();
+        if (!canonicalDocBasePrefix.endsWith(File.separator)) {
+            canonicalDocBasePrefix += File.separator;
+        }
+        JarURLConnection juc = (JarURLConnection) war.openConnection();
+        juc.setUseCaches(false);
+        JarFile jarFile = null;
+        try {
+            jarFile = juc.getJarFile();
+            Enumeration jarEntries = jarFile.entries();
+            while (jarEntries.hasMoreElements()) {
+                JarEntry jarEntry = (JarEntry) jarEntries.nextElement();
+                String name = jarEntry.getName();
+                File expandedFile = new File(docBase, name);
+                if (!expandedFile.getCanonicalPath().startsWith(
+                        canonicalDocBasePrefix)) {
+                    // Entry located outside the docBase
+                    // Throw an exception to stop the deployment
+                    throw new IllegalArgumentException(
+                            sm.getString("expandWar.illegalPath",war, name));
+                }
+            }
+        } catch (IOException e) {
+            throw e;
+        } finally {
+            if (jarFile != null) {
+                try {
+                    jarFile.close();
+                } catch (Throwable t) {
+                    // Ignore
+                }
+                jarFile = null;
+            }
+        }
+    }
+
+
     /**
      * Copy the specified file or directory to the destination.
      *
@@ -254,26 +335,61 @@ public static boolean copy(File src, File dest) {
     
     /**
      * Delete the specified directory, including all of its contents and
-     * subdirectories recursively.
+     * sub-directories recursively. Any failure will be logged.
      *
      * @param dir File object representing the directory to be deleted
      */
     public static boolean delete(File dir) {
+        // Log failure by default
+        return delete(dir, true);
+    }
+
+    /**
+     * Delete the specified directory, including all of its contents and
+     * sub-directories recursively.
+     *
+     * @param dir File object representing the directory to be deleted
+     * @param logFailure <code>true</code> if failure to delete the resource
+     *                   should be logged
+     */
+    public static boolean delete(File dir, boolean logFailure) {
+        boolean result;
         if (dir.isDirectory()) {
-            return deleteDir(dir);
+            result = deleteDir(dir, logFailure);
         } else {
-            return dir.delete();
+            if (dir.exists()) {
+                result = dir.delete();
+            } else {
+                result = true;
+            }
         }
+        if (logFailure && !result) {
+            log.error(sm.getString(
+                    "expandWar.deleteFailed", dir.getAbsolutePath()));
+        }
+        return result;
     }
     
     
     /**
      * Delete the specified directory, including all of its contents and
-     * subdirectories recursively.
+     * sub-directories recursively. Any failure will be logged.
      *
      * @param dir File object representing the directory to be deleted
      */
     public static boolean deleteDir(File dir) {
+        return deleteDir(dir, true);
+    }
+
+    /**
+     * Delete the specified directory, including all of its contents and
+     * sub-directories recursively.
+     *
+     * @param dir File object representing the directory to be deleted
+     * @param logFailure <code>true</code> if failure to delete the resource
+     *                   should be logged
+     */
+    public static boolean deleteDir(File dir, boolean logFailure) {
 
         String files[] = dir.list();
         if (files == null) {
@@ -282,12 +398,25 @@ public static boolean deleteDir(File dir) {
         for (int i = 0; i < files.length; i++) {
             File file = new File(dir, files[i]);
             if (file.isDirectory()) {
-                deleteDir(file);
+                deleteDir(file, logFailure);
             } else {
                 file.delete();
             }
         }
-        return dir.delete();
+
+        boolean result;
+        if (dir.exists()) {
+            result = dir.delete();
+        } else {
+            result = true;
+        }
+        
+        if (logFailure && !result) {
+            log.error(sm.getString(
+                    "expandWar.deleteFailed", dir.getAbsolutePath()));
+        }
+        
+        return result;
 
     }
 
@@ -302,11 +431,27 @@ public static boolean deleteDir(File dir) {
      * @return A handle to the expanded File
      *
      * @exception IOException if an input/output error occurs
+     * 
+     * @deprecated
      */
     protected static File expand(InputStream input, File docBase, String name)
         throws IOException {
-
         File file = new File(docBase, name);
+        expand(input, file);
+        return file;
+    }
+
+
+    /**
+     * Expand the specified input stream into the specified file.
+     *
+     * @param input InputStream to be copied
+     * @param file The file to be created
+     *
+     * @exception IOException if an input/output error occurs
+     */
+    private static void expand(InputStream input, File file)
+        throws IOException {
         BufferedOutputStream output = null;
         try {
             output = 
@@ -327,8 +472,6 @@ protected static File expand(InputStream input, File docBase, String name)
                 }
             }
         }
-
-        return file;
     }
 
 

@@ -26,7 +26,9 @@
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
@@ -150,6 +152,12 @@ public class HostConfig
     protected static Digester digester = createDigester();
 
 
+    /**
+     * The list of Wars in the appBase to be ignored because they are invalid
+     * (e.g. contain /../ sequences).
+     */
+    protected Set invalidWars = new HashSet();
+
     // ------------------------------------------------------------- Properties
 
 
@@ -702,13 +710,22 @@ protected void deployWARs(File appBase, String[] files) {
             if (files[i].equalsIgnoreCase("WEB-INF"))
                 continue;
             File dir = new File(appBase, files[i]);
-            if (files[i].toLowerCase().endsWith(".war")) {
+            if (files[i].toLowerCase().endsWith(".war") && dir.isFile() &&
+                    !invalidWars.contains(files[i])) {
                 
                 // Calculate the context path and make sure it is unique
                 String contextPath = "/" + files[i].replace('#','/');
                 int period = contextPath.lastIndexOf(".");
-                if (period >= 0)
-                    contextPath = contextPath.substring(0, period);
+                contextPath = contextPath.substring(0, period);
+                
+                // Check for WARs with /../ /./ or similar sequences in the name
+                if (!validateContextPath(appBase, contextPath)) {
+                    log.error(sm.getString(
+                            "hostConfig.illegalWarName", files[i]));
+                    invalidWars.add(files[i]);
+                    continue;
+                }
+
                 if (contextPath.equals("/ROOT"))
                     contextPath = "";
                 
@@ -726,6 +743,42 @@ protected void deployWARs(File appBase, String[] files) {
     }
 
 
+    private boolean validateContextPath(File appBase, String contextPath) {
+        // More complicated than the ideal as the canonical path may or may
+        // not end with File.separator for a directory
+        
+        StringBuilder docBase;
+        String canonicalDocBase = null;
+        
+        try {
+            String canonicalAppBase = appBase.getCanonicalPath();
+            docBase = new StringBuilder(canonicalAppBase);
+            if (canonicalAppBase.endsWith(File.separator)) {
+                docBase.append(contextPath.substring(1).replace(
+                        '/', File.separatorChar));
+            } else {
+                docBase.append(contextPath.replace('/', File.separatorChar));
+            }
+            // At this point docBase should be canonical but will not end
+            // with File.separator
+            
+            canonicalDocBase =
+                (new File(docBase.toString())).getCanonicalPath();
+    
+            // If the canoncialDocBase ends with File.separator, add one to
+            // docBase before they are compared
+            if (canonicalDocBase.endsWith(File.separator)) {
+                docBase.append(File.separator);
+            }
+        } catch (IOException ioe) {
+            return false;
+        }
+        
+        // Compare the two. If they are not the same, the contextPath must
+        // have /../ like sequences in it 
+        return canonicalDocBase.equals(docBase.toString());
+    }
+
     /**
      * @param contextPath
      * @param dir

@@ -57,6 +57,8 @@ engineConfig.cce=Lifecycle event data object {0} is not an Engine
 engineConfig.start=EngineConfig: Processing START
 engineConfig.stop=EngineConfig: Processing STOP
 expandWar.copy=Error copying {0} to {1}
+expandWar.deleteFailed=[{0}] could not be completely deleted. The presence of the remaining files may cause problems
+expandWar.illegalPath=The archive [{0}] is malformed and will be ignored: an entry contains an illegal path [{1}]
 hostConfig.appBase=Application base directory {0} does not exist
 hostConfig.canonicalizing=Error delete redeploy resources from context [{0}]
 hostConfig.cce=Lifecycle event data object {0} is not a Host
@@ -76,6 +78,7 @@ hostConfig.deploying=Deploying discovered web applications
 hostConfig.expand=Expanding web application archive {0}
 hostConfig.expand.error=Exception while expanding web application archive {0}
 hostConfig.expanding=Expanding discovered web application archives
+hostConfig.illegalWarName=The war name [{0}] is invalid. The archive will be ignored.
 hostConfig.jmx.register=Register context [{0}] failed
 hostConfig.jmx.unregister=Unregister context [{0}] failed
 hostConfig.reload=Reloading context [{0}]

@@ -110,6 +110,13 @@
         Do not swallow exceptions in ApplicationContextFacade.doPrivileged()
         (kkolinko)
       </fix>
+      <fix>
+        Various related (un)deploy improvements including: better handling of
+        failed (un)deployment; adding checking for invalid zip file entries that
+        don't make sense in a WAR file; and improved validation of WAR file
+        names. These changes address CVE-2009-2693, CVE-2009-2901 and
+        CVE-2009-2902.
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

@@ -29,6 +29,7 @@ standardLoader.reloading=Reloading checks are enabled for this Context
 standardLoader.removeRepository=Removing repository {0}
 standardLoader.starting=Starting this Loader
 standardLoader.stopping=Stopping this Loader
+webappClassLoader.illegalJarPath=Illegal JAR entry detected with name {0}
 webappClassLoader.jdbcRemoveFailed=JDBC driver de-registration failed
 webappClassLoader.jdbcRemoveStreamError=Exception closing input stream during JDBC driver de-registration
 webappClassLoader.stopped=Illegal access: this web application instance has been stopped already.  Could not load {0}.  The eventual following stack trace is caused by an error thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access, and has no functional impact.
@@ -39,6 +40,7 @@ webappClassLoader.clearRmiFail=Failed to clear context class loader referenced f
 webappClassLoader.clearThreadLocal=A web application created a ThreadLocal with key of type [{0}] (value [{1}]) and a value of type [{2}] (value [{3}]) but failed to remove it when the web application was stopped. To prevent a memory leak, the ThreadLocal has been forcibly removed.
 webappClassLoader.clearThreadLocalFail=Failed to clear ThreadLocal references
 webappClassLoader.stopThreadFail=Failed to terminate thread named [{0}]
+webappClassLoader.validationErrorJarPath=Unable to validate JAR entry with name {0}
 webappClassLoader.warnThread=A web application appears to have started a thread named [{0}] but has failed to stop it. This is very likely to create a memory leak. 
 webappClassLoader.wrongVersion=(unable to load class {0})
 webappLoader.addRepository=Adding repository {0}

@@ -362,7 +362,7 @@ protected boolean removeEldestEntry(
      * Path where resources loaded from JARs will be extracted.
      */
     protected File loaderDir = null;
-
+    protected String canonicalLoaderDir = null;
 
     /**
      * The PermissionCollection for each CodeSource for a web
@@ -577,6 +577,18 @@ public void setJarPath(String jarPath) {
      */
     public void setWorkDir(File workDir) {
         this.loaderDir = new File(workDir, "loader");
+        if (loaderDir == null) {
+            canonicalLoaderDir = null;
+        } else { 
+            try {
+                canonicalLoaderDir = loaderDir.getCanonicalPath();
+                if (!canonicalLoaderDir.endsWith(File.separator)) {
+                    canonicalLoaderDir += File.separator;
+                }
+            } catch (IOException ioe) {
+                canonicalLoaderDir = null;
+            }
+        }
     }
 
      /**
@@ -2514,6 +2526,18 @@ protected ResourceEntry findResourceInternal(String name, String path) {
                                         (".class"))) {
                                     resourceFile = new File
                                         (loaderDir, jarEntry2.getName());
+                                    try {
+                                        if (!resourceFile.getCanonicalPath().startsWith(
+                                                canonicalLoaderDir)) {
+                                            throw new IllegalArgumentException(
+                                                    sm.getString("webappClassLoader.illegalJarPath",
+                                                jarEntry2.getName()));
+                                        }
+                                    } catch (IOException ioe) {
+                                        throw new IllegalArgumentException(
+                                                sm.getString("webappClassLoader.validationErrorJarPath",
+                                                        jarEntry2.getName()), ioe);
+                                    }                                 
                                     resourceFile.getParentFile().mkdirs();
                                     FileOutputStream os = null;
                                     InputStream is = null;

@@ -733,35 +733,40 @@ protected void fixDocBase()
         file = new File(docBase);
         String origDocBase = docBase;
         
-        String contextPath = context.getPath();
-        if (contextPath.equals("")) {
-            contextPath = "ROOT";
+        String pathName = context.getPath();
+        if (pathName.equals("")) {
+            pathName = "ROOT";
         } else {
-            if (contextPath.lastIndexOf('/') > 0) {
-                contextPath = "/" + contextPath.substring(1).replace('/','#');
-            }
+            // Context path must start with '/'
+            pathName = pathName.substring(1).replace('/', '#');
         }
         if (docBase.toLowerCase().endsWith(".war") && !file.isDirectory() && unpackWARs) {
             URL war = new URL("jar:" + (new File(docBase)).toURI().toURL() + "!/");
-            docBase = ExpandWar.expand(host, war, contextPath);
+            docBase = ExpandWar.expand(host, war, pathName);
             file = new File(docBase);
             docBase = file.getCanonicalPath();
             if (context instanceof StandardContext) {
                 ((StandardContext) context).setOriginalDocBase(origDocBase);
             }
+        } else if (docBase.toLowerCase().endsWith(".war") &&
+                !file.isDirectory() && !unpackWARs) {
+            URL war =
+                new URL("jar:" + (new File (docBase)).toURI().toURL() + "!/");
+            ExpandWar.validate(host, war, pathName);
         } else {
             File docDir = new File(docBase);
             if (!docDir.exists()) {
                 File warFile = new File(docBase + ".war");
                 if (warFile.exists()) {
+                    URL war =
+                        new URL("jar:" + warFile.toURI().toURL() + "!/");
                     if (unpackWARs) {
-                        URL war =
-                            new URL("jar:" + warFile.toURI().toURL() + "!/");
-                        docBase = ExpandWar.expand(host, war, contextPath);
+                        docBase = ExpandWar.expand(host, war, pathName);
                         file = new File(docBase);
                         docBase = file.getCanonicalPath();
                     } else {
                         docBase = warFile.getCanonicalPath();
+                        ExpandWar.validate(host, war, pathName);
                     }
                 }
                 if (context instanceof StandardContext) {
@@ -1122,7 +1127,8 @@ protected synchronized void stop() {
             if (!docBaseFile.isAbsolute()) {
                 docBaseFile = new File(appBase, docBase);
             }
-            ExpandWar.delete(docBaseFile);
+            // No need to log failure - it is expected in this case
+            ExpandWar.delete(docBaseFile, false);
         }
         
         ok = true;

@@ -66,7 +66,8 @@ public class ExpandWar {
      *  (must start with "jar:")
      * @param pathname Context path name for web application
      *
-     * @exception IllegalArgumentException if this is not a "jar:" URL
+     * @exception IllegalArgumentException if this is not a "jar:" URL or if the
+     *            WAR file is invalid
      * @exception IOException if an input/output error was encountered
      *  during expansion
      */
@@ -84,6 +85,7 @@ public static String expand(Host host, URL war, String pathname)
                 (sm.getString("hostConfig.appBase",
                               appBase.getAbsolutePath()));
         }
+        
         File docBase = new File(appBase, pathname);
         if (docBase.exists()) {
             // War file is already installed
@@ -94,16 +96,29 @@ public static String expand(Host host, URL war, String pathname)
         docBase.mkdir();
 
         // Expand the WAR into the new document base directory
+        String canonicalDocBasePrefix = docBase.getCanonicalPath();
+        if (!canonicalDocBasePrefix.endsWith(File.separator)) {
+            canonicalDocBasePrefix += File.separator;
+        }
         JarURLConnection juc = (JarURLConnection) war.openConnection();
         juc.setUseCaches(false);
         JarFile jarFile = null;
         InputStream input = null;
+        boolean success = false;
         try {
             jarFile = juc.getJarFile();
             Enumeration<JarEntry> jarEntries = jarFile.entries();
             while (jarEntries.hasMoreElements()) {
                 JarEntry jarEntry = jarEntries.nextElement();
                 String name = jarEntry.getName();
+                File expandedFile = new File(docBase, name);
+                if (!expandedFile.getCanonicalPath().startsWith(
+                        canonicalDocBasePrefix)) {
+                    // Trying to expand outside the docBase
+                    // Throw an exception to stop the deployment
+                    throw new IllegalArgumentException(
+                            sm.getString("expandWar.illegalPath",war, name));
+                }
                 int last = name.lastIndexOf('/');
                 if (last >= 0) {
                     File parent = new File(docBase,
@@ -116,21 +131,24 @@ public static String expand(Host host, URL war, String pathname)
                 input = jarFile.getInputStream(jarEntry);
 
                 // Bugzilla 33636
-                File expandedFile = expand(input, docBase, name);
+                expand(input, expandedFile);
                 long lastModified = jarEntry.getTime();
-                if ((lastModified != -1) && (lastModified != 0) && (expandedFile != null)) {
+                if ((lastModified != -1) && (lastModified != 0)) {
                     expandedFile.setLastModified(lastModified);
                 }
 
                 input.close();
                 input = null;
             }
+            success = true;
         } catch (IOException e) {
-            // If something went wrong, delete expanded dir to keep things 
-            // clean
-            deleteDir(docBase);
             throw e;
         } finally {
+            if (!success) {
+                // If something went wrong, delete expanded dir to keep things 
+                // clean
+                deleteDir(docBase);
+            }
             if (input != null) {
                 try {
                     input.close();
@@ -155,6 +173,69 @@ public static String expand(Host host, URL war, String pathname)
     }
 
 
+    /**
+     * Validate the WAR file found at the specified URL.
+     *
+     * @param host Host war is being installed for
+     * @param war URL of the web application archive to be validated
+     *  (must start with "jar:")
+     * @param pathname Context path name for web application
+     *
+     * @exception IllegalArgumentException if this is not a "jar:" URL or if the
+     *            WAR file is invalid
+     * @exception IOException if an input/output error was encountered
+     *            during validation
+     */
+    public static void validate(Host host, URL war, String pathname)
+        throws IOException {
+
+        // Make the appBase absolute
+        File appBase = new File(host.getAppBase());
+        if (!appBase.isAbsolute()) {
+            appBase = new File(System.getProperty("catalina.base"),
+                               host.getAppBase());
+        }
+        
+        File docBase = new File(appBase, pathname);
+
+        // Calculate the document base directory
+        String canonicalDocBasePrefix = docBase.getCanonicalPath();
+        if (!canonicalDocBasePrefix.endsWith(File.separator)) {
+            canonicalDocBasePrefix += File.separator;
+        }
+        JarURLConnection juc = (JarURLConnection) war.openConnection();
+        juc.setUseCaches(false);
+        JarFile jarFile = null;
+        try {
+            jarFile = juc.getJarFile();
+            Enumeration<JarEntry> jarEntries = jarFile.entries();
+            while (jarEntries.hasMoreElements()) {
+                JarEntry jarEntry = jarEntries.nextElement();
+                String name = jarEntry.getName();
+                File expandedFile = new File(docBase, name);
+                if (!expandedFile.getCanonicalPath().startsWith(
+                        canonicalDocBasePrefix)) {
+                    // Entry located outside the docBase
+                    // Throw an exception to stop the deployment
+                    throw new IllegalArgumentException(
+                            sm.getString("expandWar.illegalPath",war, name));
+                }
+            }
+        } catch (IOException e) {
+            throw e;
+        } finally {
+            if (jarFile != null) {
+                try {
+                    jarFile.close();
+                } catch (Throwable t) {
+                    // Ignore
+                }
+                jarFile = null;
+            }
+        }
+    }
+
+
     /**
      * Copy the specified file or directory to the destination.
      *
@@ -215,26 +296,61 @@ public static boolean copy(File src, File dest) {
     
     /**
      * Delete the specified directory, including all of its contents and
-     * subdirectories recursively.
+     * sub-directories recursively. Any failure will be logged.
      *
      * @param dir File object representing the directory to be deleted
      */
     public static boolean delete(File dir) {
+        // Log failure by default
+        return delete(dir, true);
+    }
+
+    /**
+     * Delete the specified directory, including all of its contents and
+     * sub-directories recursively.
+     *
+     * @param dir File object representing the directory to be deleted
+     * @param logFailure <code>true</code> if failure to delete the resource
+     *                   should be logged
+     */
+    public static boolean delete(File dir, boolean logFailure) {
+        boolean result;
         if (dir.isDirectory()) {
-            return deleteDir(dir);
+            result = deleteDir(dir, logFailure);
         } else {
-            return dir.delete();
+            if (dir.exists()) {
+                result = dir.delete();
+            } else {
+                result = true;
+            }
         }
+        if (logFailure && !result) {
+            log.error(sm.getString(
+                    "expandWar.deleteFailed", dir.getAbsolutePath()));
+        }
+        return result;
     }
     
     
     /**
      * Delete the specified directory, including all of its contents and
-     * subdirectories recursively.
+     * sub-directories recursively. Any failure will be logged.
      *
      * @param dir File object representing the directory to be deleted
      */
     public static boolean deleteDir(File dir) {
+        return deleteDir(dir, true);
+    }
+
+    /**
+     * Delete the specified directory, including all of its contents and
+     * sub-directories recursively.
+     *
+     * @param dir File object representing the directory to be deleted
+     * @param logFailure <code>true</code> if failure to delete the resource
+     *                   should be logged
+     */
+    public static boolean deleteDir(File dir, boolean logFailure) {
 
         String files[] = dir.list();
         if (files == null) {
@@ -243,12 +359,25 @@ public static boolean deleteDir(File dir) {
         for (int i = 0; i < files.length; i++) {
             File file = new File(dir, files[i]);
             if (file.isDirectory()) {
-                deleteDir(file);
+                deleteDir(file, logFailure);
             } else {
                 file.delete();
             }
         }
-        return dir.delete();
+
+        boolean result;
+        if (dir.exists()) {
+            result = dir.delete();
+        } else {
+            result = true;
+        }
+        
+        if (logFailure && !result) {
+            log.error(sm.getString(
+                    "expandWar.deleteFailed", dir.getAbsolutePath()));
+        }
+        
+        return result;
 
     }
 
@@ -263,11 +392,27 @@ public static boolean deleteDir(File dir) {
      * @return A handle to the expanded File
      *
      * @exception IOException if an input/output error occurs
+     * 
+     * @deprecated
      */
     protected static File expand(InputStream input, File docBase, String name)
         throws IOException {
-
         File file = new File(docBase, name);
+        expand(input, file);
+        return file;
+    }
+
+
+    /**
+     * Expand the specified input stream into the specified file.
+     *
+     * @param input InputStream to be copied
+     * @param file The file to be created
+     *
+     * @exception IOException if an input/output error occurs
+     */
+    private static void expand(InputStream input, File file)
+        throws IOException {
         BufferedOutputStream output = null;
         try {
             output = 
@@ -288,8 +433,6 @@ protected static File expand(InputStream input, File docBase, String name)
                 }
             }
         }
-
-        return file;
     }
 
 

@@ -28,7 +28,9 @@
 import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
@@ -154,6 +156,11 @@ public class HostConfig
      */
     protected static Digester digester = createDigester();
 
+    /**
+     * The list of Wars in the appBase to be ignored because they are invalid
+     * (e.g. contain /../ sequences).
+     */
+    protected Set<String> invalidWars = new HashSet<String>();
 
     // ------------------------------------------------------------- Properties
 
@@ -715,13 +722,22 @@ protected void deployWARs(File appBase, String[] files) {
             if (files[i].equalsIgnoreCase("WEB-INF"))
                 continue;
             File dir = new File(appBase, files[i]);
-            if (files[i].toLowerCase().endsWith(".war") && dir.isFile()) {
+            if (files[i].toLowerCase().endsWith(".war") && dir.isFile()
+                    && !invalidWars.contains(files[i]) ) {
                 
                 // Calculate the context path and make sure it is unique
                 String contextPath = "/" + files[i].replace('#','/');
                 int period = contextPath.lastIndexOf(".");
-                if (period >= 0)
-                    contextPath = contextPath.substring(0, period);
+                contextPath = contextPath.substring(0, period);
+                
+                // Check for WARs with /../ /./ or similar sequences in the name
+                if (!validateContextPath(appBase, contextPath)) {
+                    log.error(sm.getString(
+                            "hostConfig.illegalWarName", files[i]));
+                    invalidWars.add(files[i]);
+                    continue;
+                }
+
                 if (contextPath.equals("/ROOT"))
                     contextPath = "";
                 
@@ -739,6 +755,42 @@ protected void deployWARs(File appBase, String[] files) {
     }
 
 
+    private boolean validateContextPath(File appBase, String contextPath) {
+        // More complicated than the ideal as the canonical path may or may
+        // not end with File.separator for a directory
+        
+        StringBuilder docBase;
+        String canonicalDocBase = null;
+        
+        try {
+            String canonicalAppBase = appBase.getCanonicalPath();
+            docBase = new StringBuilder(canonicalAppBase);
+            if (canonicalAppBase.endsWith(File.separator)) {
+                docBase.append(contextPath.substring(1).replace(
+                        '/', File.separatorChar));
+            } else {
+                docBase.append(contextPath.replace('/', File.separatorChar));
+            }
+            // At this point docBase should be canonical but will not end
+            // with File.separator
+            
+            canonicalDocBase =
+                (new File(docBase.toString())).getCanonicalPath();
+    
+            // If the canoncialDocBase ends with File.separator, add one to
+            // docBase before they are compared
+            if (canonicalDocBase.endsWith(File.separator)) {
+                docBase.append(File.separator);
+            }
+        } catch (IOException ioe) {
+            return false;
+        }
+        
+        // Compare the two. If they are not the same, the contextPath must
+        // have /../ like sequences in it 
+        return canonicalDocBase.equals(docBase.toString());
+    }
+
     /**
      * @param contextPath
      * @param war

@@ -63,6 +63,8 @@ engineConfig.cce=Lifecycle event data object {0} is not an Engine
 engineConfig.start=EngineConfig: Processing START
 engineConfig.stop=EngineConfig: Processing STOP
 expandWar.copy=Error copying {0} to {1}
+expandWar.deleteFailed=[{0}] could not be completely deleted. The presence of the remaining files may cause problems
+expandWar.illegalPath=The archive [{0}] is malformed and will be ignored: an entry contains an illegal path [{1}]
 hostConfig.appBase=Application base directory {0} does not exist
 hostConfig.canonicalizing=Error delete redeploy resources from context [{0}]
 hostConfig.cce=Lifecycle event data object {0} is not a Host
@@ -83,6 +85,7 @@ hostConfig.deploying=Deploying discovered web applications
 hostConfig.expand=Expanding web application archive {0}
 hostConfig.expand.error=Exception while expanding web application archive {0}
 hostConfig.expanding=Expanding discovered web application archives
+hostConfig.illegalWarName=The war name [{0}] is invalid. The archive will be ignored.
 hostConfig.jmx.register=Register context [{0}] failed
 hostConfig.jmx.unregister=Unregister context [{0}] failed
 hostConfig.reload=Reloading context [{0}]