Vincent Massol wrote:
> Hi Sergiu,
>
> On Mar 11, 2008, at 3:01 AM, sdumitriu (SVN) wrote:
>
>> Author: sdumitriu
>> Date: 2008-03-11 03:01:30 +0100 (Tue, 11 Mar 2008)
>> New Revision: 8341
>>
>> Modified:
>> xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/
>> XWiki.java
>> Log:
>> XWIKI-2173
>> Fixed.
>>
>>
>> Modified: xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/
>> xwiki/XWiki.java
>> ===================================================================
>> --- xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/
>> XWiki.java 2008-03-11 00:13:35 UTC (rev 8340)
>> +++ xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/
>> XWiki.java 2008-03-11 02:01:30 UTC (rev 8341)
>> @@ -1492,9 +1492,16 @@
>> // Try parsing a file located in the directory with the same
>> name.
>> try {
>> String path = "/skins/" + skin + "/" + template;
>> - String content = getResourceContent(path);
>> - return XWikiVelocityRenderer.evaluate(content, path,
>> (VelocityContext) context
>> - .get("vcontext"), context);
>> + File f = new File(path);
>> + path = f.getCanonicalPath();
>
> Why do we need these 2 lines?
Did you read the description on XWIKI-2173?
> We really need a comment here to explain why we're checking for "/
> skins/".
Adding it now.
>> + if (path.startsWith("/skins/")) {
>> + String content = getResourceContent(path);
>> + return XWikiVelocityRenderer.evaluate(content,
>> path, (VelocityContext) context
>> + .get("vcontext"), context);
>> + } else {
>> + LOG.warn("Illegal access, tried to use file [" +
>> path + "] as a template." +
>> + " Possible break-in attempt!");
>
> I would be more neutral in the log message. Also I don't understand
> how you deduce some user is trying to use a file as a template. Cannot
> it be simply someone trying to use a skin file?
This code is in a method named parseTemplate.
> I'd use something like: "Invalid access. For security reasons only
> resources located in the skins directory are allowed."
This should NOT happen in a normal wiki with normal users. Only
specially crafter URLs can get outside of /skins/, so it is something an
admin should care about. Anyway, this message is just for the log, which
can be seen just by sysadmins. If it were a message for a user, I would
have put a nicer message. By the way, if the user is requesting for a
valid template, it will be retrieved from the next valid location (base
skin or default base skin or templates) or will receive a "template does
not exist" message.
> Thanks
> -Vincent
>
>> + }
>> } catch (Exception e) {
>> }
>
> _______________________________________________
> devs mailing list
> [email protected]
> http://lists.xwiki.org/mailman/listinfo/devs
>
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
