Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Commit

Permalink
Various related (un)deploy improvements including:
Browse files Browse the repository at this point in the history 
- better handling of failed (un)deployment
- adding checking for invalid zip file entries that don't make sense in a WAR file
- improved validation of WAR file names.
These changes address CVE-2009-2693, CVE-2009-2901 and CVE-2009-2902.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc5.5.x/trunk@902650 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@markt-asf
markt-asf committed Jan 24, 2010
1 parent 8cdc334 commit 0299cb7
Show file tree
Hide file tree
Showing 7 changed files with 269 additions and 29 deletions.
2 changes: 2 additions & 0 deletions container/catalina/src/share/org/apache/catalina/loader/LocalStrings.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
26 changes: 26 additions & 0 deletions container/catalina/src/share/org/apache/catalina/loader/WebappClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;


/**
Expand Down Expand Up @@ -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;
}
}

}

/**
Expand Down Expand Up @@ -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;
Expand Down
28 changes: 17 additions & 11 deletions container/catalina/src/share/org/apache/catalina/startup/ContextConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
173 changes: 158 additions & 15 deletions container/catalina/src/share/org/apache/catalina/startup/ExpandWar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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();
Expand All @@ -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.
*
Expand Down Expand Up @@ -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) {
Expand All @@ -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;

}

Expand All @@ -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 =
Expand All @@ -327,8 +472,6 @@ protected static File expand(InputStream input, File docBase, String name)
}
}
}

return file;
}


Expand Down
Loading

0 comments on commit 0299cb7

Please # to comment.