[
https://issues.apache.org/jira/browse/WICKET-4427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215699#comment-13215699
]
Sebastiaan van Erk commented on WICKET-4427:
--------------------------------------------
Hi,
Thanks for looking at this issue, but I have several issues with the proposed
fix.
1) It does not address the real problem: why is a file "x%20" resolved to "x"
on the file system in the first place (if I request x%20 then I expect it to
open x%20 and not x).
2) PackageResourceGuard is an implementation of an interface, namely of
IPackageResourceGuard with one method boolean accept(final Class<?> scope,
final String path). As it stands, the same problem can occur with other
implementations, and in fact, it is strange that if I accept "x%20" for
whatever reason with this *interface*, that it should suddenly return a
resource with a different path.
When debugging to find the real reason this went wrong, I found it to be
related to this code:
public ExtensionResourceNameIterator(String path, final String
extension)
{
if ((extension == null) && (path.indexOf('.') != -1))
{
// Get the extension from the path provided
extensions = new String[] { "." +
Strings.lastPathComponent(path, '.') };
path = Strings.beforeLastPathComponent(path, '.');
}
else if (extension != null)
{
// Extension can be a comma separated list
extensions = Strings.split(extension, ',');
for (int i = extensions.length - 1; i >= 0; i--)
{
extensions[i] = extensions[i].trim();
if (!extensions[i].startsWith("."))
{
extensions[i] = "." + extensions[i];
}
}
}
else
{
extensions = new String[1];
extensions[0] = ".";
}
this.path = path;
index = 0;
}
Basically, this enumerator get the real extension of the file, *and then
processes it*, *AFTER THE RESOURCE GUARD CHECK HAS ALREADY BEEN DONE*.
I don't know at which point it is necessary to initialize this iterator from a
single string containing a comma separated list of extensions, but to split
this string and process it here is doing it at the wrong level because it
cannot distinguish between 1 extension with a comma in it and multiple
extensions, but it also trims (processing which just shouldn't be done here).
The splitting should probably be being done at the point where the extension is
clearly value of configuration property.
All in all, the previous fix did not actually fix the problem, it just removed
a symptom of the problem for one implemementation of IPackageResourceGuard.
To prove this, note that the above extension processing does two things: split
on , and trim. This means that another way to subvert the check is to use the
extension ".properties,xml", i.e., using the url:
http://localhost:8080/wicket-4427/resources/test/test.properties,xml
> possible to bypass PackageResourceGuard
> ---------------------------------------
>
> Key: WICKET-4427
> URL: https://issues.apache.org/jira/browse/WICKET-4427
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.4.18
> Reporter: Sebastiaan van Erk
> Assignee: Martin Grigorov
> Fix For: 1.4.20, 1.5.5, 6.0.0
>
> Attachments: wicket-4427.zip
>
>
> It is possible to bypass the filters in the PackageResourceGuard by adding
> %10, %13, or %20 to the end of the url, i.e.,
> http://my.site.com/myapp/mounted_package/myfile.properties%20
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
