Re: [xwiki-devs] [xwiki-notifications] r8341 - xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki
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? We really need a comment here to explain why we're checking for "/ skins/".
+ 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? I'd use something like: "Invalid access. For security reasons only resources located in the skins directory are allowed." Thanks -Vincent
+ } } catch (Exception e) { }
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/
participants (2)
-
Sergiu Dumitriu -
Vincent Massol