You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by jw...@apache.org on 2007/09/14 20:46:32 UTC
svn commit: r575774 - in /myfaces/trinidad/trunk/trinidad:
trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/
trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/
trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal...
Author: jwaldman
Date: Fri Sep 14 11:44:34 2007
New Revision: 575774
URL: http://svn.apache.org/viewvc?rev=575774&view=rev
Log:
https://issues.apache.org/jira/browse/TRINIDAD-703
Make image loading more secure
The code change is in ClassLoaderResourceLoader.java.
LoggerBundle.xrts has 2 new translation strings
The other classes are comment additions
Modified:
myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/ClassLoaderResourceLoader.java
myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/RegexResourceLoader.java
myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreClassLoaderResourceLoader.java
myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreRenderKitResourceLoader.java
Modified: myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/ClassLoaderResourceLoader.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/ClassLoaderResourceLoader.java?rev=575774&r1=575773&r2=575774&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/ClassLoaderResourceLoader.java (original)
+++ myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/ClassLoaderResourceLoader.java Fri Sep 14 11:44:34 2007
@@ -22,10 +22,14 @@
import java.net.URL;
+import org.apache.myfaces.trinidad.logging.TrinidadLogger;
+
/**
* A resource loader implementation which loads resources
- * using the context class loader.
+ * using the context class loader. The returned resource URL will be null
+ * for paths that attempt to access paths outside the root directory by having
+ * ".." in the path.
*
*/
public class ClassLoaderResourceLoader extends ResourceLoader
@@ -54,6 +58,7 @@
* level resource package.
*
* @param rootPackage the top level package used to interpret resource paths
+ * For example, it could be "META-INF".
*/
public ClassLoaderResourceLoader(
String rootPackage)
@@ -95,6 +100,12 @@
path = path.substring(1);
}
+ if (!_isPathWithinRoot(path))
+ {
+ _LOG.severe("RESOURCE_PATH_OUTSIDE_ROOT", path);
+ return null;
+ }
+
return getClassLoader().getResource(path);
}
@@ -128,6 +139,65 @@
return rootPackage.replace('.', '/');
}
+
+ /**
+ * Checks to see if the path when ".." are resolved out is within the root.
+ * Returns true if the path is within the root, otherwise returns false.
+ * e.g., /afr/../../foo.gif is out of root. It gets resolved to /../foo.gif.
+ * /afr/../tmp/../xyz/foo.gif is within the root. It gets resolved to
+ * /xyz/foo.gif
+ *
+ * @param path -String the path
+ * @return boolean true if the path does not get resolved outside the root
+ */
+ private static boolean _isPathWithinRoot(String path)
+ {
+ if (path.indexOf("..") == -1)
+ return true;
+
+ // It would be strange if the path has ".." in it because
+ // the browsers (IE7 and Firefox 1.5) resolve ".." out of the path before
+ // requesting the resource.
+ // Therefore we warn if we find a path with "..".
+ // Then, we try to resolve it ourselves to find out if it is outside
+ // the root.
+ _LOG.warning("RESOURCE_PATH_DOTS", path);
+
+ while (path != null && path.length() > 0 )
+ {
+ // as soon as it starts with .. or /.., we know we are out of bounds
+ if (path.startsWith(".."))
+ return false;
+ int index = path.indexOf("/../");
+ if (index == 0)
+ return false;
+
+ // couldn't find "/../" in the path, so we are within the root
+ if (index == -1)
+ return true;
+
+ // resolve out the first match of "/../" by taking it out as well
+ // as the /foo/ before it.
+ String beginning = path.substring(0, index);
+ String end = path.substring(index+4);
+
+ int lastIndex = beginning.lastIndexOf("/");
+ if (lastIndex == -1)
+ {
+ //could not find / in the beginning string, so strip the entire thing
+ path = end;
+ }
+ else
+ {
+ path = beginning.substring(0, lastIndex+1) + end;
+ }
+ }
+
+ return true;
+ }
+
private final String _resourcePrefix;
+ private static final TrinidadLogger _LOG = TrinidadLogger.createTrinidadLogger(
+ ClassLoaderResourceLoader.class);
}
Modified: myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/RegexResourceLoader.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/RegexResourceLoader.java?rev=575774&r1=575773&r2=575774&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/RegexResourceLoader.java (original)
+++ myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/resource/RegexResourceLoader.java Fri Sep 14 11:44:34 2007
@@ -54,6 +54,13 @@
protected URL findResource(
String path) throws IOException
{
+
+ // a RegexResourceNode contains a ResourceLoader
+ // (e.g., a CoreClassLoaderResourceLoader) and a Pattern.
+ // loop through all the RegExResourceNodes that have been registered
+ // with the RegexResourceLoader's register(regex, loader) method,
+ // and find the node where the path fits the node's pattern.
+ // Once the node is found, return the resource with the given name
for(RegexResourceNode node : _loaders)
{
Matcher matcher = node.getPattern().matcher(path);
Modified: myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts?rev=575774&r1=575773&r2=575774&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts (original)
+++ myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts Fri Sep 14 11:44:34 2007
@@ -387,5 +387,10 @@
<!-- VIEW_TAG_NOT_PRESENT -->
<resource key="VIEW_TAG_NOT_PRESENT"><f:view> was not present on this page; tag {0} encountered without an <f:view> being processed.</resource>
+ <!-- RESOURCE_PATH_OUTSIDE_ROOT -->
+ <resource key="RESOURCE_PATH_OUTSIDE_ROOT">The resource path {0} contains ".." and it resolves to be outside the root directory, and that is insecure. Therefore the path is ignored.</resource>
+
+ <!-- RESOURCE_PATH_CONTAINS_DOTS -->
+ <resource key="RESOURCE_PATH_DOTS">The resource path {0} contains "..". Browsers resolve out the "..", so this is a suspicious path.</resource>
</resources>
Modified: myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreClassLoaderResourceLoader.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreClassLoaderResourceLoader.java?rev=575774&r1=575773&r2=575774&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreClassLoaderResourceLoader.java (original)
+++ myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreClassLoaderResourceLoader.java Fri Sep 14 11:44:34 2007
@@ -26,7 +26,9 @@
/**
- *
+ * A resource loader implementation which loads resources
+ * using the context class loader with a rootPackage of META-INF. This will
+ * find the resources within the META-INF directory.
*/
public class CoreClassLoaderResourceLoader extends ClassLoaderResourceLoader
{
@@ -35,10 +37,15 @@
*/
public CoreClassLoaderResourceLoader(ResourceLoader parent)
{
- super("META-INF", parent);
+ super("META-INF", parent);
}
- @Override
+
+
+ /**
+ * Override to pull out the version from the path.
+ */
+ @Override
protected URL findResource(
String path) throws IOException
{
Modified: myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreRenderKitResourceLoader.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreRenderKitResourceLoader.java?rev=575774&r1=575773&r2=575774&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreRenderKitResourceLoader.java (original)
+++ myfaces/trinidad/trunk/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/resource/CoreRenderKitResourceLoader.java Fri Sep 14 11:44:34 2007
@@ -37,7 +37,7 @@
/**
* A resource loader implementation which loads resources
- * for the rich renderkit.
+ * for the core renderkit.
*
*
* @todo Dynamic version number
@@ -53,7 +53,7 @@
new CoreCommonScriptsResourceLoader(_getCommonLibraryURI(true),
true));
register("(/.*LocaleElements.*\\.js)",
- new LocaleElementsResourceLoader(getLocaleElementsURI("LocaleElements", true)));
+ new LocaleElementsResourceLoader(getLocaleElementsURI("LocaleElements", true)));
register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
new CoreClassLoaderResourceLoader(parent));