[jboss-cvs] JBossAS SVN: r62874 - in branches/JBoss_4_0_5_GA_CP: testsuite/src/main/org/jboss/test and 2 other directories.
jboss-cvs-commits at lists.jboss.org
jboss-cvs-commits at lists.jboss.org
Tue May 8 11:14:50 EDT 2007
Author: dbhole
Date: 2007-05-08 11:14:50 -0400 (Tue, 08 May 2007)
New Revision: 62874
Added:
branches/JBoss_4_0_5_GA_CP/testsuite/src/main/org/jboss/test/console/
branches/JBoss_4_0_5_GA_CP/testsuite/src/main/org/jboss/test/console/jbas3861/
branches/JBoss_4_0_5_GA_CP/testsuite/src/main/org/jboss/test/console/jbas3861/JBAS3861UnitTestCase.java
Modified:
branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepository.java
branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepositoryMBean.java
Log:
Fix for JBAS-3861: DeploymentFileRepository can be used to write/remove arbitrary files in the filesystem
Modified: branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepository.java
===================================================================
--- branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepository.java 2007-05-08 14:55:31 UTC (rev 62873)
+++ branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepository.java 2007-05-08 15:14:50 UTC (rev 62874)
@@ -21,18 +21,18 @@
*/
package org.jboss.console.manager;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.PrintWriter;
+
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+
import org.jboss.system.ServiceMBeanSupport;
import org.jboss.system.server.ServerConfig;
import org.jboss.system.server.ServerConfigLocator;
-import javax.management.ObjectName;
-import javax.management.MBeanServer;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.PrintWriter;
-import java.io.IOException;
-import java.net.URL;
-
/**
* This class wraps the file system
* for deployments. It gives a file-based
@@ -44,16 +44,17 @@
* corresponds to the base file name.
*
* @author <a href="mailto:bill at jboss.org">Bill Burke</a>
+ * @author <a href="mailto:dimitris at jboss.org">Dimitris Andreadis</a>
* @version $Revision$
- *
- **/
-public class DeploymentFileRepository extends ServiceMBeanSupport implements DeploymentFileRepositoryMBean
+ */
+public class DeploymentFileRepository extends ServiceMBeanSupport
+ implements DeploymentFileRepositoryMBean
{
private String baseDir;
private File base;
+
/** The server's home directory, for relative paths. */
protected File serverHome;
- protected URL serverHomeURL;
/**
*
@@ -67,8 +68,8 @@
public void store(String folder, String name, String fileExtension, String data, boolean noHotDeploy) throws IOException
{
log.debug("store called");
- File dir = new File(base, folder);
- log.debug("respository folder: " + dir.toString());
+ File dir = getFile(base, folder);
+ log.debug("repository folder: " + dir.toString());
log.debug("absolute: " + dir.getAbsolutePath());
if (!dir.exists())
{
@@ -78,11 +79,13 @@
}
}
String filename = name.replace(' ', '_') + fileExtension;
- File file = new File(dir, filename);
+ File file = getFile(dir, filename);
+
File tmpfile = new File(dir, filename + ".tmp");
PrintWriter writer = new PrintWriter(new FileOutputStream(tmpfile));
writer.write(data);
writer.close();
+
if (file.exists() && noHotDeploy)
{
long modified = file.lastModified();
@@ -95,19 +98,19 @@
}
}
- public void remove(String folder, String name, String fileExtension)
+ public void remove(String folder, String name, String fileExtension) throws IOException
{
- File dir = new File(base, folder);
+ File dir = getFile(base, folder);
String filename = name.replace(' ', '_') + fileExtension;
- File file = new File(dir, filename);
+ File file = getFile(dir, filename);
file.delete();
}
- public boolean isStored(String folder, String name, String fileExtension)
+ public boolean isStored(String folder, String name, String fileExtension) throws IOException
{
- File dir = new File(base, folder);
+ File dir = getFile(base, folder);
String filename = name.replace(' ', '_') + fileExtension;
- File file = new File(dir, filename);
+ File file = getFile(dir, filename);
return file.exists();
}
@@ -116,15 +119,15 @@
return baseDir;
}
- public void setBaseDir(String baseDir)
+ public void setBaseDir(String baseDir) throws IOException
{
+ this.base = getFile(serverHome, baseDir);
this.baseDir = baseDir;
- this.base = new File(serverHome, baseDir);
+
+ log.debug("BaseDir set to: " + this.base);
}
-
- public ObjectName preRegister(MBeanServer server, ObjectName name)
- throws Exception
+ public ObjectName preRegister(MBeanServer server, ObjectName name) throws Exception
{
// get server's home for relative paths, need this for setting
// attribute final values, so we need to do it here
@@ -133,4 +136,20 @@
return super.preRegister(server, name);
}
+ /**
+ * Wrap the File(File parent, String child) CTOR to make sure the
+ * resulting child is indeed under the parent hierarchy,
+ * i.e. don't allow a ../../../rogue-child.txt
+ *
+ * see JBAS-3861
+ */
+ private File getFile(File parent, String child) throws IOException
+ {
+ File childFile = new File(parent, child);
+
+ if (childFile.getCanonicalPath().indexOf(parent.getCanonicalPath()) != 0)
+ throw new IllegalArgumentException("child '" + child + "' should be a child of parent '" + parent + "'");
+
+ return childFile;
+ }
}
Modified: branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepositoryMBean.java
===================================================================
--- branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepositoryMBean.java 2007-05-08 14:55:31 UTC (rev 62873)
+++ branches/JBoss_4_0_5_GA_CP/console/src/main/org/jboss/console/manager/DeploymentFileRepositoryMBean.java 2007-05-08 15:14:50 UTC (rev 62874)
@@ -21,26 +21,31 @@
*/
package org.jboss.console.manager;
+import java.io.IOException;
+
import org.jboss.system.ServiceMBean;
-import java.io.IOException;
-
/**
- * Comment
+ * MBean interface
*
* @author <a href="mailto:bill at jboss.org">Bill Burke</a>
+ * @author <a href="mailto:dimitris at jboss.org">Dimitris Andreadis</a>
* @version $Revision$
- *
- **/
+ */
public interface DeploymentFileRepositoryMBean extends ServiceMBean
{
+ // Attributes ----------------------------------------------------
+
+ /** The base directory to use for storing/removing files */
+ void setBaseDir(String baseDir) throws IOException;
+ String getBaseDir();
+
+ // Operations ----------------------------------------------------
+
void store(String folder, String name, String fileExtension, String data, boolean noHotDeploy) throws IOException;
- void remove(String folder, String name, String fileExtension);
+ void remove(String folder, String name, String fileExtension) throws IOException;
- boolean isStored(String folder, String name, String fileExtension);
+ boolean isStored(String folder, String name, String fileExtension) throws IOException;
- String getBaseDir();
-
- void setBaseDir(String baseDir);
}
Added: branches/JBoss_4_0_5_GA_CP/testsuite/src/main/org/jboss/test/console/jbas3861/JBAS3861UnitTestCase.java
===================================================================
--- branches/JBoss_4_0_5_GA_CP/testsuite/src/main/org/jboss/test/console/jbas3861/JBAS3861UnitTestCase.java (rev 0)
+++ branches/JBoss_4_0_5_GA_CP/testsuite/src/main/org/jboss/test/console/jbas3861/JBAS3861UnitTestCase.java 2007-05-08 15:14:50 UTC (rev 62874)
@@ -0,0 +1,106 @@
+/*
+ * JBoss, Home of Professional Open Source.
+ * Copyright 2006, Red Hat Middleware LLC, and individual contributors
+ * as indicated by the @author tags. See the copyright.txt file in the
+ * distribution for a full listing of individual contributors.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.jboss.test.console.jbas3861;
+
+import javax.management.Attribute;
+import javax.management.ObjectName;
+import javax.management.RuntimeMBeanException;
+
+import org.jboss.mx.util.ObjectNameFactory;
+import org.jboss.test.JBossTestCase;
+
+/**
+ * Test JBAS-3861 (DeploymentFileRepository service)
+ *
+ * @author <a href="mailto:dimitris at jboss.org">Dimitris Andreadis</a>
+ * @version $Revision: 57211 $
+ */
+public class JBAS3861UnitTestCase extends JBossTestCase
+{
+ ObjectName target= ObjectNameFactory.create("jboss.admin:service=DeploymentFileRepository");
+
+ public JBAS3861UnitTestCase(String name)
+ {
+ super(name);
+ }
+
+ /**
+ * Check if BaseDir can be set outside the server home directory
+ */
+ public void testSetBaseDirOutsideServerHomeDir() throws Exception
+ {
+ // remember original BaseDir
+ String basedir = (String)getServer().getAttribute(target, "BaseDir");
+ try
+ {
+ // Should throw an IllegalArgumentException
+ getServer().setAttribute(target, new Attribute("BaseDir", ".."));
+ // Should throw an IllegalArgumentException
+ getServer().setAttribute(target, new Attribute("BaseDir", "/"));
+
+ // Restore the original dir and fail the test
+ getServer().setAttribute(target, new Attribute("BaseDir", basedir));
+ fail("Managed to set BaseDir outside ServerHomeDir for service: " + target);
+ }
+ catch (RuntimeMBeanException e)
+ {
+ // expected
+ }
+ }
+
+ /**
+ * Check if we can write a file outside the server home directory
+ */
+ public void testStoreFileOutsideServerHomeDir() throws Exception
+ {
+ try
+ {
+ // Should throw an exception
+ getServer().invoke(
+ target,
+ "store",
+ new Object[] { "..", "jbas3861", ".tmp", "file content", Boolean.TRUE },
+ new String[] { "java.lang.String", "java.lang.String", "java.lang.String", "java.lang.String", Boolean.TYPE.toString() });
+
+ // Should throw an exception
+ getServer().invoke(
+ target,
+ "store",
+ new Object[] { ".", "../jbas3861", ".tmp", "file content", Boolean.TRUE },
+ new String[] { "java.lang.String", "java.lang.String", "java.lang.String", "java.lang.String", Boolean.TYPE.toString() });
+
+ // Remove the stored file and fail the test - normally it should throw an exception, too
+ getServer().invoke(
+ target,
+ "remove",
+ new Object[] { ".", "../jbas3861", ".tmp" },
+ new String[] { "java.lang.String", "java.lang.String", "java.lang.String" });
+
+ fail("Managed to create/remove a file outside ServerHomeDir for service: " + target);
+ }
+ catch (RuntimeMBeanException e)
+ {
+ // expected
+ }
+ }
+
+}
More information about the jboss-cvs-commits
mailing list