You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Jeanne Waldman (JIRA)" <de...@myfaces.apache.org> on 2007/09/12 00:17:32 UTC
[jira] Created: (TRINIDAD-703) Make image loading more secure
Make image loading more secure
------------------------------
Key: TRINIDAD-703
URL: https://issues.apache.org/jira/browse/TRINIDAD-703
Project: MyFaces Trinidad
Issue Type: Bug
Reporter: Jeanne Waldman
Assignee: Jeanne Waldman
Andy Schwartz found this issue:
We register our image resource loader with a fairly loose pattern:
register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
new CoreClassLoaderResourceLoader(parent));
In theory could someone get at an image on the class path outside of our own
images by doing crafting a funky URL along the lines of
"../../../../oracle/someotherpackage/foo.gif"?
ClassLoaderResourceLoader
should prevent access outside of the "rootPackage".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TRINIDAD-703) Make image loading more secure
Posted by "Jeanne Waldman (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/TRINIDAD-703?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526623 ]
Jeanne Waldman commented on TRINIDAD-703:
-----------------------------------------
I also noticed in DirectoryResourceLoader this code that does not do what it is intended to do:
* A resource loader implementation which loads resources
* from a directory. The returned resource URL will be null
* for file resources that do not exist, or for relative paths
* that attempt to access paths outside the root directory.
// "root" directory path should always be less than the file path
boolean isContained = (_directory.compareTo(file) <= 0);
I want to change this to compare canonical paths:
// file path should contain the "root" directory path, not be outside it
boolean isContained = file.getCanonicalPath().startsWith(_directoryPath);
> Make image loading more secure
> ------------------------------
>
> Key: TRINIDAD-703
> URL: https://issues.apache.org/jira/browse/TRINIDAD-703
> Project: MyFaces Trinidad
> Issue Type: Bug
> Reporter: Jeanne Waldman
> Assignee: Jeanne Waldman
>
> Andy Schwartz found this issue:
> We register our image resource loader with a fairly loose pattern:
> register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> new CoreClassLoaderResourceLoader(parent));
> In theory could someone get at an image on the class path outside of our own
> images by doing crafting a funky URL along the lines of
> "../../../../oracle/someotherpackage/foo.gif"?
> ClassLoaderResourceLoader
> should prevent access outside of the "rootPackage".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TRINIDAD-703) Make image loading more secure
Posted by "Jeanne Waldman (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/TRINIDAD-703?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526624 ]
Jeanne Waldman commented on TRINIDAD-703:
-----------------------------------------
In ClassLoaderResoureLoader, I want to add this code to fix the problem:
// if the path has ".." in it, then log an ERROR and return null
if (path.indexOf("..") > -1)
{
_LOG.severe("The resource's path {0} contains \"..\" which is illegal, so " +
"it will be ignored: ", path);
return null;
}
Please comment if there are any issues with this.
> Make image loading more secure
> ------------------------------
>
> Key: TRINIDAD-703
> URL: https://issues.apache.org/jira/browse/TRINIDAD-703
> Project: MyFaces Trinidad
> Issue Type: Bug
> Reporter: Jeanne Waldman
> Assignee: Jeanne Waldman
>
> Andy Schwartz found this issue:
> We register our image resource loader with a fairly loose pattern:
> register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> new CoreClassLoaderResourceLoader(parent));
> In theory could someone get at an image on the class path outside of our own
> images by doing crafting a funky URL along the lines of
> "../../../../oracle/someotherpackage/foo.gif"?
> ClassLoaderResourceLoader
> should prevent access outside of the "rootPackage".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TRINIDAD-703) Make image loading more secure
Posted by "Jeanne Waldman (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/TRINIDAD-703?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12527576 ]
Jeanne Waldman commented on TRINIDAD-703:
-----------------------------------------
It turns out that Firefox and IE both resolve urls that have ".." in them to take out the "..". Even css background-image urls.
Therefore, it should be suspicious if we get a ".." in the path.
I plan to:
1. warn if I see ".." in the path, because this should never happen
2. if "..", then figure out if the resolved path lies within the root or not
3. severe error if the resolved path is outside the root.
> Make image loading more secure
> ------------------------------
>
> Key: TRINIDAD-703
> URL: https://issues.apache.org/jira/browse/TRINIDAD-703
> Project: MyFaces Trinidad
> Issue Type: Bug
> Reporter: Jeanne Waldman
> Assignee: Jeanne Waldman
>
> Andy Schwartz found this issue:
> We register our image resource loader with a fairly loose pattern:
> register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> new CoreClassLoaderResourceLoader(parent));
> In theory could someone get at an image on the class path outside of our own
> images by doing crafting a funky URL along the lines of
> "../../../../oracle/someotherpackage/foo.gif"?
> ClassLoaderResourceLoader
> should prevent access outside of the "rootPackage".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (TRINIDAD-703) Make image loading more secure
Posted by "Jeanne Waldman (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/TRINIDAD-703?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526658 ]
Jeanne Waldman commented on TRINIDAD-703:
-----------------------------------------
Will I break anyone if I reject any paths that have .. in them rather than figuring out what the entire path with '..' resolve to and make sure it is still under the root?
> Make image loading more secure
> ------------------------------
>
> Key: TRINIDAD-703
> URL: https://issues.apache.org/jira/browse/TRINIDAD-703
> Project: MyFaces Trinidad
> Issue Type: Bug
> Reporter: Jeanne Waldman
> Assignee: Jeanne Waldman
>
> Andy Schwartz found this issue:
> We register our image resource loader with a fairly loose pattern:
> register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> new CoreClassLoaderResourceLoader(parent));
> In theory could someone get at an image on the class path outside of our own
> images by doing crafting a funky URL along the lines of
> "../../../../oracle/someotherpackage/foo.gif"?
> ClassLoaderResourceLoader
> should prevent access outside of the "rootPackage".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (TRINIDAD-703) Make image loading more secure
Posted by "Jeanne Waldman (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/TRINIDAD-703?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jeanne Waldman resolved TRINIDAD-703.
-------------------------------------
Resolution: Fixed
The code change is in ClassLoaderResourceLoader.java.
LoggerBundle.xrts has 2 new translation strings
The other classes are comment additions
svn Completed: At revision: 575774
trinidad-api\src\main\java\org\apache\myfaces\trinidad\resource\ClassLoaderResourceLoader.java
trinidad-api\src\main\java\org\apache\myfaces\trinidad\resource\RegexResourceLoader.java
trinidad-api\src\main\xrts\org\apache\myfaces\trinidad\resource\LoggerBundle.xrts
trinidad-impl\src\main\java\org\apache\myfaces\trinidadinternal\resource\CoreClassLoaderResourceLoader.java
trinidad-impl\src\main\java\org\apache\myfaces\trinidadinternal\resource\CoreRenderKitResourceLoader.java
trinidad-api\src\main\java\org\apache\myfaces\trinidad\resource\ClassLoaderResourceLoader.java
trinidad-impl\src\main\java\org\apache\myfaces\trinidadinternal\resource\CoreRenderKitResourceLoader.java
trinidad-api\src\main\xrts\org\apache\myfaces\trinidad\resource\LoggerBundle.xrts
trinidad-api\src\main\java\org\apache\myfaces\trinidad\resource\RegexResourceLoader.java
trinidad-impl\src\main\java\org\apache\myfaces\trinidadinternal\resource\CoreClassLoaderResourceLoader.java
> Make image loading more secure
> ------------------------------
>
> Key: TRINIDAD-703
> URL: https://issues.apache.org/jira/browse/TRINIDAD-703
> Project: MyFaces Trinidad
> Issue Type: Bug
> Reporter: Jeanne Waldman
> Assignee: Jeanne Waldman
>
> Andy Schwartz found this issue:
> We register our image resource loader with a fairly loose pattern:
> register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> new CoreClassLoaderResourceLoader(parent));
> In theory could someone get at an image on the class path outside of our own
> images by doing crafting a funky URL along the lines of
> "../../../../oracle/someotherpackage/foo.gif"?
> ClassLoaderResourceLoader
> should prevent access outside of the "rootPackage".
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.