From 0e6833b7707d677c9991e5b30dad4f0a68bab8dd Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 12 Jul 2016 11:39:58 -0700 Subject: [PATCH] Issue #687 - Simplifying AllowSymLinkAliasChecker logic --- .../handler/AllowSymLinkAliasChecker.java | 84 +++++++------------ 1 file changed, 30 insertions(+), 54 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java index 278a89831bc..a5d6bf016c6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java @@ -29,9 +29,11 @@ import org.eclipse.jetty.util.resource.Resource; /* ------------------------------------------------------------ */ -/** Symbolic Link AliasChecker. + +/** + * Symbolic Link AliasChecker. *

An instance of this class can be registered with {@link ContextHandler#addAliasCheck(AliasCheck)} - * to check resources that are aliased to other locations. The checker uses the + * to check resources that are aliased to other locations. The checker uses the * Java {@link Files#readSymbolicLink(Path)} and {@link Path#toRealPath(java.nio.file.LinkOption...)} * APIs to check if a file is aliased with symbolic links.

*/ @@ -46,7 +48,7 @@ public class AllowSymLinkAliasChecker implements AliasCheck if (!(resource instanceof PathResource)) return false; - PathResource pathResource = (PathResource)resource; + PathResource pathResource = (PathResource) resource; try { @@ -56,62 +58,14 @@ public class AllowSymLinkAliasChecker implements AliasCheck if (path.equals(alias)) return false; // Unknown why this is an alias - // is the file itself a symlink? - if (Files.isSymbolicLink(path)) - { - alias = path.getParent().resolve(alias); - if (LOG.isDebugEnabled()) - { - LOG.debug("path ={}",path); - LOG.debug("alias={}",alias); - } - if (Files.isSameFile(path,alias)) - { - if (LOG.isDebugEnabled()) - LOG.debug("Allow symlink {} --> {}",resource,pathResource.getAliasPath()); - return true; - } - } - - // No, so let's check each element ourselves - boolean linked=true; - Path target=path; - int loops=0; - while (linked) - { - if (++loops>100) - { - if (LOG.isDebugEnabled()) - LOG.debug("Too many symlinks {} --> {}",resource,target); - return false; - } - linked=false; - Path d = target.getRoot(); - for (Path e:target) - { - Path r=d.resolve(e); - d=r; - - while (Files.exists(d) && Files.isSymbolicLink(d)) - { - Path link=Files.readSymbolicLink(d); - if (!link.isAbsolute()) - link=d.getParent().resolve(link).normalize(); - d=link; - linked=true; - } - } - target=d; - } - - if (pathResource.getAliasPath().equals(target)) + if (hasSymbolicLink(path) && Files.isSameFile(path, alias)) { if (LOG.isDebugEnabled()) - LOG.debug("Allow path symlink {} --> {}",resource,target); + LOG.debug("Allow symlink {} --> {}", resource, pathResource.getAliasPath()); return true; } } - catch(Exception e) + catch (Exception e) { LOG.ignore(e); } @@ -119,4 +73,26 @@ public class AllowSymLinkAliasChecker implements AliasCheck return false; } + private boolean hasSymbolicLink(Path path) + { + // Is file itself a symlink? + if (Files.isSymbolicLink(path)) + { + return true; + } + + // Lets try each path segment + Path base = path.getRoot(); + for (Path segment : path) + { + base = base.resolve(segment); + if (Files.isSymbolicLink(base)) + { + return true; + } + } + + return false; + } + }