From 7a4bf22b1b77bfbccf0c5bc0de82347fe7508d29 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Wed, 7 Oct 2026 16:07:37 -0400 Subject: [PATCH] fix(webdav): confine temp storage writes to the temp directory WebDAV temp-storage paths and names come from the client. Resolve every temp path strictly inside the temp directory (the directory itself is not a valid resource), and require names that reach temp storage to be a single plain path segment. Temp writes (copy, move, create file, create folder) build their target through one helper and reject invalid input with 400 before touching the filesystem. Rejections are logged without the request path. Adds DotWebdavTempPathContainmentTest, registered in MainSuite3a. --- .../webdav/BasicFolderResourceImpl.java | 17 +- .../dotmarketing/webdav/DotWebdavHelper.java | 118 ++++++++++- .../webdav/FolderResourceImpl.java | 44 ++++- .../dotmarketing/webdav/HostResourceImpl.java | 28 ++- .../webdav/LanguageFolderResourceImpl.java | 30 ++- .../webdav/TempFileResourceImpl.java | 29 ++- .../webdav/TempFolderResourceImpl.java | 83 ++++++-- .../src/test/java/com/dotcms/MainSuite3a.java | 3 +- .../DotWebdavTempPathContainmentTest.java | 187 ++++++++++++++++++ 9 files changed, 498 insertions(+), 41 deletions(-) create mode 100644 dotcms-integration/src/test/java/com/dotmarketing/webdav/DotWebdavTempPathContainmentTest.java diff --git a/dotCMS/src/main/java/com/dotmarketing/webdav/BasicFolderResourceImpl.java b/dotCMS/src/main/java/com/dotmarketing/webdav/BasicFolderResourceImpl.java index fc0dc4ebd263..4f5912d7e12d 100644 --- a/dotCMS/src/main/java/com/dotmarketing/webdav/BasicFolderResourceImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/webdav/BasicFolderResourceImpl.java @@ -49,7 +49,21 @@ public BasicFolderResourceImpl(String path) { this.isAutoPub=dotDavHelper.isAutoPub(path); } - public Resource createNew(String newName, final InputStream in, final Long length, final String contentType) throws IOException, DotRuntimeException { + /** + * Creates a file named {@code newName} in this folder. A temp-style name goes to temp storage + * and must be a single plain path segment; any other name is stored as content. + * + * @param newName the file name received from the WebDAV client + * @param in the file content + * @param length the content length, if known + * @param contentType the content type, if known + * @return the created resource + * @throws IOException if the content cannot be read or written + * @throws DotRuntimeException if the file cannot be created + * @throws BadRequestException (HTTP 400) when a temp-style name is not a single plain path segment + */ + public Resource createNew(String newName, final InputStream in, final Long length, final String contentType) + throws IOException, DotRuntimeException, BadRequestException { if(newName.matches("^\\.(.*)-Spotlight$")){ // http://jira.dotmarketing.net/browse/DOTCMS-7285 @@ -83,6 +97,7 @@ public Resource createNew(String newName, final InputStream in, final Long lengt } } else { + DotWebdavHelper.requirePlainSegment(this, newName); return this.createNewTemporalResource(newName, in); } } // createNew. diff --git a/dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java b/dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java index c08834e0cb35..5e1862c9babf 100644 --- a/dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java +++ b/dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java @@ -15,6 +15,7 @@ import com.bradmcevoy.http.Request; import com.bradmcevoy.http.Request.Method; import com.bradmcevoy.http.Resource; +import com.bradmcevoy.http.exceptions.BadRequestException; import com.dotmarketing.beans.Host; import com.dotmarketing.beans.Identifier; import com.dotmarketing.beans.Permission; @@ -391,6 +392,15 @@ public Folder loadFolder(String url,User user) throws IOException{ return folder; } + /** + * Resolves a temporary WebDAV resource from a request-derived path, confined to the temp + * directory. The path is built under {@link #getTempDir()} and then checked for containment: + * a path that resolves outside the temp root (for example through a {@code ..} traversal) is + * rejected and {@code null} is returned, so the caller treats it as "not found". + * + * @param url the request URL (endpoint mapping is stripped internally) + * @return the temp {@link File}, or {@code null} when the path escapes the temp root + */ public File loadTempFile(String url){ try { url = stripMapping(url); @@ -398,7 +408,12 @@ public File loadTempFile(String url){ Logger.error( this, "Error happened with uri: [" + url + "]", e); } Logger.debug(this, "Getting temp file from path " + url); - return new File(getTempDir().getPath() + url); + final File tempFile = new File(getTempDir().getPath() + url); + if (!isWithinTempDir(tempFile)) { + Logger.warn(this, "Rejected a WebDAV temp path outside the temp directory"); + return null; + } + return tempFile; } /** @@ -507,6 +522,81 @@ public File getTempDir () { return new File(ConfigUtils.getAssetTempPath()); } + /** + * Verifies that {@code candidate} resolves to a location strictly inside the WebDAV temp + * directory ({@link #getTempDir()}). The temp directory itself is not a valid temp resource. + * Canonicalization collapses {@code .} and {@code ..} segments and resolves symlinks, so the + * check holds however the path is written. Fails closed: any canonicalization error is treated + * as "not contained". + * + * @param candidate the {@link File} built from request-derived input + * @return {@code true} only when the canonical candidate is a descendant of the temp directory + */ + private boolean isWithinTempDir(final File candidate) { + try { + final String root = getTempDir().getCanonicalPath(); + return candidate.getCanonicalPath().startsWith(root + File.separator); + } catch (IOException e) { + Logger.error(this, "WebDAV temp-path canonicalization failed; rejecting", e); + return false; + } + } + + /** + * Tells whether a client-supplied name can be used as a child name in temp storage: a single + * plain path segment that is not empty, not {@code .} or {@code ..}, and contains no path + * separator or NUL character. + * + * @param name the name received from the WebDAV client + * @return {@code true} when the name is a single plain path segment + */ + static boolean isPlainSegment(final String name) { + return name != null + && !name.isEmpty() + && !".".equals(name) + && !"..".equals(name) + && name.indexOf('/') < 0 + && name.indexOf('\\') < 0 + && name.indexOf('\0') < 0; + } + + /** + * Rejects a client-supplied name that is not a single plain path segment, before it reaches + * temp storage. + * + * @param resource the resource handling the request, reported in the rejection + * @param name the name received from the WebDAV client + * @throws BadRequestException (HTTP 400) when the name is not a single plain path segment + */ + static void requirePlainSegment(final Resource resource, final String name) throws BadRequestException { + if (!isPlainSegment(name)) { + Logger.warn(DotWebdavHelper.class, "Rejected a WebDAV name that is not a single path segment"); + throw new BadRequestException(resource, "Invalid name"); + } + } + + /** + * Resolves the target of a write into temp storage: {@code name} inside {@code folder}. The name + * must be a single plain path segment and the target must stay strictly inside the temp + * directory. + * + * @param resource the resource handling the request, reported in the rejection + * @param folder the temp folder receiving the write + * @param name the name received from the WebDAV client + * @return the target {@link File} + * @throws BadRequestException (HTTP 400) when the name or the target is not valid + */ + File resolveTempChild(final Resource resource, final File folder, final String name) + throws BadRequestException { + requirePlainSegment(resource, name); + final File target = new File(folder, name); + if (!isWithinTempDir(target)) { + Logger.warn(this, "Rejected a WebDAV temp target outside the temp directory"); + throw new BadRequestException(resource, "Invalid destination"); + } + return target; + } + public String getHostName ( String uri ) { try { return getHostname(stripMapping(uri)); @@ -523,6 +613,15 @@ public boolean isTempResource(String path){ return false; } + /** + * Creates a temporary WebDAV folder under {@link #getTempDir()}, confined to the temp directory. + * A {@code path} that resolves outside the temp root (for example through a {@code ..} traversal) + * is rejected before any directory is created. + * + * @param path the request-derived folder path (endpoint mapping is stripped internally) + * @return the created temp folder + * @throws DotRuntimeException if the path escapes the temp root + */ public File createTempFolder(String path){ try { path = stripMapping(path); @@ -536,6 +635,10 @@ public File createTempFolder(String path){ } path = path.replace("/", File.separator); File f = new File(getTempDir().getPath() + File.separator + path); + if (!isWithinTempDir(f)) { + Logger.warn(this, "Rejected a WebDAV temp folder outside the temp directory"); + throw new DotRuntimeException("WebDAV temp folder path escapes the temp root"); + } f.mkdirs(); return f; } @@ -575,8 +678,21 @@ public File copyFileToTemp(IFileAsset file, File tempFolder) throws IOException{ return nf; } + /** + * Creates a temporary WebDAV file under {@link #getTempDir()}, confined to the temp directory. + * A {@code path} that resolves outside the temp root (for example through a {@code ..} traversal) + * is rejected before any file or parent directory is created. + * + * @param path the request-derived file path, relative to the temp root + * @return the created temp file + * @throws IOException if the path escapes the temp root, or the file cannot be created + */ public File createTempFile(String path) throws IOException{ File file = new File(getTempDir().getPath() + path); + if (!isWithinTempDir(file)) { + Logger.warn(this, "Rejected a WebDAV temp file outside the temp directory"); + throw new IOException("WebDAV temp file path escapes the temp root"); + } String p = file.getPath().substring(0,file.getPath().lastIndexOf(File.separator)); File f = new File(p); f.mkdirs(); diff --git a/dotCMS/src/main/java/com/dotmarketing/webdav/FolderResourceImpl.java b/dotCMS/src/main/java/com/dotmarketing/webdav/FolderResourceImpl.java index c9beeb8b8969..bf335b2dff88 100644 --- a/dotCMS/src/main/java/com/dotmarketing/webdav/FolderResourceImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/webdav/FolderResourceImpl.java @@ -55,14 +55,21 @@ public FolderResourceImpl(Folder folder, String path) { this.hostAPI = APILocator.getHostAPI(); } - /* (non-Javadoc) - * @see com.bradmcevoy.http.MakeCollectionableResource#createCollection(java.lang.String) + /** + * Creates a folder named {@code newName} inside this folder. A temp-style name goes to temp + * storage and must be a single plain path segment. + * + * @param newName the folder name received from the WebDAV client + * @return the created folder + * @throws DotRuntimeException if the folder cannot be created + * @throws BadRequestException (HTTP 400) when a temp-style name is not a single plain path segment */ - public CollectionResource createCollection(String newName) throws DotRuntimeException { + public CollectionResource createCollection(String newName) throws DotRuntimeException, BadRequestException { User user=(User)HttpManager.request().getAuthorization().getTag(); String folderPath =""; if(dotDavHelper.isTempResource(newName)){ + DotWebdavHelper.requirePlainSegment(this, newName); Host host; try { host = hostAPI.find(folder.getHostId(), user, false); @@ -259,6 +266,7 @@ public void copyTo(CollectionResource collRes, String name) throws NotAuthorized User user=(User)HttpManager.request().getAuthorization().getTag(); if(collRes instanceof TempFolderResourceImpl){ + DotWebdavHelper.requirePlainSegment(this, name); TempFolderResourceImpl tr = (TempFolderResourceImpl)collRes; try { dotDavHelper.copyFolderToTemp(folder, tr.getFolder(), user, name, isAutoPub, lang); @@ -289,13 +297,20 @@ public void copyTo(CollectionResource collRes, String name) throws NotAuthorized } } - /* (non-Javadoc) - * @see com.bradmcevoy.http.MoveableResource#moveTo(com.bradmcevoy.http.CollectionResource, java.lang.String) + /** + * Moves this folder into {@code collRes} under {@code name}. A move that ends in temp storage + * requires {@code name} to be a single plain path segment. + * + * @param collRes the destination collection + * @param name the destination name received from the WebDAV client + * @throws DotRuntimeException if the move fails + * @throws BadRequestException (HTTP 400) when a temp destination name is not a single plain path segment */ - public void moveTo(CollectionResource collRes, String name) throws DotRuntimeException{ + public void moveTo(CollectionResource collRes, String name) throws DotRuntimeException, BadRequestException { User user=(User)HttpManager.request().getAuthorization().getTag(); if(collRes instanceof TempFolderResourceImpl){ Logger.debug(this, "Webdav clients wants to move a file from dotcms to a temporary storage but we don't allow this in fear that the transaction may break and delete a file from dotcms"); + DotWebdavHelper.requirePlainSegment(this, name); TempFolderResourceImpl tr = (TempFolderResourceImpl)collRes; try { dotDavHelper.copyFolderToTemp(folder, tr.getFolder(), user, name, isAutoPub, lang); @@ -306,6 +321,7 @@ public void moveTo(CollectionResource collRes, String name) throws DotRuntimeExc }else if(collRes instanceof FolderResourceImpl){ FolderResourceImpl fr = (FolderResourceImpl)collRes; if(dotDavHelper.isTempResource(name)){ + DotWebdavHelper.requirePlainSegment(this, name); Host host; String folderPath = ""; try { @@ -401,9 +417,23 @@ public Long getMaxAgeSeconds(Auth arg0) { } + /** + * Creates a folder named {@code name} and locks it. The lock interface only allows + * {@link NotAuthorizedException}, so an invalid temp-style name is reported as a runtime error. + * + * @param name the folder name received from the WebDAV client + * @param timeout the requested lock timeout + * @param lockInfo the requested lock details + * @return the lock token of the created folder + * @throws NotAuthorizedException if the lock is not authorized + */ public LockToken createAndLock(String name, LockTimeout timeout, LockInfo lockInfo) throws NotAuthorizedException { - createCollection(name); + try { + createCollection(name); + } catch (BadRequestException e) { + throw new DotRuntimeException(e.getReason(), e); + } return lock(timeout, lockInfo).getLockToken(); } diff --git a/dotCMS/src/main/java/com/dotmarketing/webdav/HostResourceImpl.java b/dotCMS/src/main/java/com/dotmarketing/webdav/HostResourceImpl.java index 72f7cc78b99e..63832bdd8cf4 100644 --- a/dotCMS/src/main/java/com/dotmarketing/webdav/HostResourceImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/webdav/HostResourceImpl.java @@ -252,9 +252,19 @@ public Date getCreateDate() { return host.getModDate(); } - public CollectionResource createCollection(String newName) throws DotRuntimeException { + /** + * Creates a folder named {@code newName} at the root of this site. A temp-style name goes to + * temp storage and must be a single plain path segment. + * + * @param newName the folder name received from the WebDAV client + * @return the created folder + * @throws DotRuntimeException if the folder cannot be created + * @throws BadRequestException (HTTP 400) when a temp-style name is not a single plain path segment + */ + public CollectionResource createCollection(String newName) throws DotRuntimeException, BadRequestException { User user=(User)HttpManager.request().getAuthorization().getTag(); if(dotDavHelper.isTempResource(newName)){ + DotWebdavHelper.requirePlainSegment(this, newName); File f = dotDavHelper.createTempFolder(File.separator + host.getHostname() + File.separator + newName); TempFolderResourceImpl tr = new TempFolderResourceImpl(f.getPath(),f ,isAutoPub); return tr; @@ -275,9 +285,23 @@ public CollectionResource createCollection(String newName) throws DotRuntimeExce } } + /** + * Creates a folder named {@code name} and locks it. The lock interface only allows + * {@link NotAuthorizedException}, so an invalid temp-style name is reported as a runtime error. + * + * @param name the folder name received from the WebDAV client + * @param timeout the requested lock timeout + * @param lockInfo the requested lock details + * @return the lock token of the created folder + * @throws NotAuthorizedException if the lock is not authorized + */ public LockToken createAndLock(String name, LockTimeout timeout, LockInfo lockInfo) throws NotAuthorizedException { - createCollection(name); + try { + createCollection(name); + } catch (BadRequestException e) { + throw new DotRuntimeException(e.getReason(), e); + } return lock(timeout, lockInfo).getLockToken(); } diff --git a/dotCMS/src/main/java/com/dotmarketing/webdav/LanguageFolderResourceImpl.java b/dotCMS/src/main/java/com/dotmarketing/webdav/LanguageFolderResourceImpl.java index b580ba1f8cb7..94aa94d9eefb 100644 --- a/dotCMS/src/main/java/com/dotmarketing/webdav/LanguageFolderResourceImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/webdav/LanguageFolderResourceImpl.java @@ -15,7 +15,9 @@ import com.bradmcevoy.http.Request; import com.bradmcevoy.http.Request.Method; import com.bradmcevoy.http.Resource; +import com.bradmcevoy.http.exceptions.BadRequestException; import com.bradmcevoy.http.exceptions.NotAuthorizedException; +import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.business.Role; import com.dotmarketing.business.web.WebAPILocator; import com.dotmarketing.util.Config; @@ -81,11 +83,17 @@ public LanguageFolderResourceImpl(String path) { } } - /* (non-Javadoc) - * @see com.bradmcevoy.http.MakeCollectionableResource#createCollection(java.lang.String) + /** + * Creates a temp folder for a temp-style {@code newName} at the language root; any other name is + * not supported here. The temp-style name must be a single plain path segment. + * + * @param newName the folder name received from the WebDAV client + * @return the created temp folder, or {@code null} when the name is not supported here + * @throws BadRequestException (HTTP 400) when a temp-style name is not a single plain path segment */ - public CollectionResource createCollection(String newName) { + public CollectionResource createCollection(String newName) throws BadRequestException { if(dotDavHelper.isTempResource(newName) && isLanguageRoot){ + DotWebdavHelper.requirePlainSegment(this, newName); dotDavHelper.createTempFolder(File.separator + "system" + File.separator + "languages" + File.separator + newName); File f = new File(File.separator + "system" + File.separator + "languages"); TempFolderResourceImpl tr = new TempFolderResourceImpl(f.getPath(),f ,true); @@ -374,9 +382,23 @@ public Long getMaxAgeSeconds(Auth arg0) { return (long)60; } + /** + * Creates a temp folder named {@code name} and locks it. The lock interface only allows + * {@link NotAuthorizedException}, so an invalid temp-style name is reported as a runtime error. + * + * @param name the folder name received from the WebDAV client + * @param timeout the requested lock timeout + * @param lockInfo the requested lock details + * @return the lock token of the created folder + * @throws NotAuthorizedException if the lock is not authorized + */ public LockToken createAndLock(String name, LockTimeout timeout, LockInfo lockInfo) throws NotAuthorizedException { - createCollection(name); + try { + createCollection(name); + } catch (BadRequestException e) { + throw new DotRuntimeException(e.getReason(), e); + } return lock(timeout, lockInfo).getLockToken(); } diff --git a/dotCMS/src/main/java/com/dotmarketing/webdav/TempFileResourceImpl.java b/dotCMS/src/main/java/com/dotmarketing/webdav/TempFileResourceImpl.java index 9d3224f1d94a..973ffdeb803b 100644 --- a/dotCMS/src/main/java/com/dotmarketing/webdav/TempFileResourceImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/webdav/TempFileResourceImpl.java @@ -13,6 +13,7 @@ import com.bradmcevoy.http.Range; import com.bradmcevoy.http.Request; import com.bradmcevoy.http.Resource; +import com.bradmcevoy.http.exceptions.BadRequestException; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.PermissionAPI; import com.dotmarketing.util.Config; @@ -147,12 +148,22 @@ public Long getMaxAgeSeconds() { } - public void copyTo(CollectionResource collRes, String name) { + /** + * Copies this temp file into {@code collRes} under {@code name}. The name must be a single plain + * path segment, and a copy into temp storage must stay inside the temp directory. + * + * @param collRes the destination collection + * @param name the destination name received from the WebDAV client + * @throws BadRequestException (HTTP 400) when the name or the temp target is not valid + */ + public void copyTo(CollectionResource collRes, String name) throws BadRequestException { + DotWebdavHelper.requirePlainSegment(this, name); User user=(User)HttpManager.request().getAuthorization().getTag(); if(collRes instanceof TempFolderResourceImpl){ TempFolderResourceImpl tr = (TempFolderResourceImpl)collRes; + final File target = dotDavHelper.resolveTempChild(this, tr.getFolder(), name); try { - FileUtil.copyFile(file, new File(tr.getFolder().getPath() + File.separator + name)); + FileUtil.copyFile(file, target); } catch (Exception e) { Logger.error(this, e.getMessage(), e); return; @@ -191,12 +202,22 @@ public void delete() { } - public void moveTo(CollectionResource collRes, String name) { + /** + * Moves this temp file into {@code collRes} under {@code name}. The name must be a single plain + * path segment, and a move into temp storage must stay inside the temp directory. + * + * @param collRes the destination collection + * @param name the destination name received from the WebDAV client + * @throws BadRequestException (HTTP 400) when the name or the temp target is not valid + */ + public void moveTo(CollectionResource collRes, String name) throws BadRequestException { + DotWebdavHelper.requirePlainSegment(this, name); User user=(User)HttpManager.request().getAuthorization().getTag(); if(collRes instanceof TempFolderResourceImpl){ TempFolderResourceImpl tr = (TempFolderResourceImpl)collRes; + final File target = dotDavHelper.resolveTempChild(this, tr.getFolder(), name); try { - FileUtil.copyFile(file, new File(tr.getFolder().getPath() + File.separator + name)); + FileUtil.copyFile(file, target); file.delete(); } catch (Exception e) { Logger.error(this, e.getMessage(), e); diff --git a/dotCMS/src/main/java/com/dotmarketing/webdav/TempFolderResourceImpl.java b/dotCMS/src/main/java/com/dotmarketing/webdav/TempFolderResourceImpl.java index a6ce67a1c76a..e76eeb3a0d54 100644 --- a/dotCMS/src/main/java/com/dotmarketing/webdav/TempFolderResourceImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/webdav/TempFolderResourceImpl.java @@ -17,7 +17,9 @@ import com.bradmcevoy.http.Request; import com.bradmcevoy.http.Request.Method; import com.bradmcevoy.http.Resource; +import com.bradmcevoy.http.exceptions.BadRequestException; import com.bradmcevoy.http.exceptions.NotAuthorizedException; +import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.util.Logger; import com.liferay.portal.model.User; import com.liferay.util.FileUtil; @@ -50,14 +52,18 @@ public TempFolderResourceImpl(String path, File folder, boolean isAutoPub) { this.folder = folder; } - /* (non-Javadoc) - * @see com.bradmcevoy.http.MakeCollectionableResource#createCollection(java.lang.String) + /** + * Creates a sub-folder named {@code newName} inside this temp folder. The name must be a single + * plain path segment and the new folder must stay inside the temp directory. + * + * @param newName the folder name received from the WebDAV client + * @return the created temp folder + * @throws BadRequestException (HTTP 400) when the name or the target is not valid */ - public CollectionResource createCollection(String newName) { - dotDavHelper.createTempFolder(folder.getPath() + File.separator + newName); - File f = new File(folder.getPath() + File.separator + newName); - TempFolderResourceImpl tr = new TempFolderResourceImpl(folder.getPath() + File.separator + newName,f, isAutoPub); - return tr; + public CollectionResource createCollection(String newName) throws BadRequestException { + final File f = dotDavHelper.resolveTempChild(this, folder, newName); + dotDavHelper.createTempFolder(f.getPath()); + return new TempFolderResourceImpl(f.getPath(), f, isAutoPub); } /* (non-Javadoc) @@ -164,11 +170,21 @@ public String getUniqueId() { return folder.hashCode() + ""; } - /* (non-Javadoc) - * @see com.bradmcevoy.http.PutableResource#createNew(java.lang.String, java.io.InputStream, java.lang.Long, java.lang.String) + /** + * Writes a new temp file named {@code newName} inside this temp folder. The name must be a single + * plain path segment and the file must stay inside the temp directory. + * + * @param newName the file name received from the WebDAV client + * @param in the file content + * @param length the content length, if known + * @param contentType the content type, if known + * @return the written temp file + * @throws IOException if the file cannot be written + * @throws BadRequestException (HTTP 400) when the name or the target is not valid */ - public Resource createNew(String newName, InputStream in, Long length, String contentType) throws IOException { - File f = new File(folder.getPath() + File.separator + newName); + public Resource createNew(String newName, InputStream in, Long length, String contentType) + throws IOException, BadRequestException { + File f = dotDavHelper.resolveTempChild(this, folder, newName); if(!f.exists()){ String p = f.getPath().substring(0,f.getPath().lastIndexOf(File.separator)); File fe = new File(p); @@ -186,16 +202,21 @@ public Resource createNew(String newName, InputStream in, Long length, String co return tr; } - /* (non-Javadoc) - * @see com.bradmcevoy.http.CopyableResource#copyTo(com.bradmcevoy.http.CollectionResource, java.lang.String) + /** + * Copies this temp folder into {@code collRes} under {@code name}. The name must be a single plain + * path segment, and a copy into temp storage must stay inside the temp directory. + * + * @param collRes the destination collection + * @param name the destination name received from the WebDAV client + * @throws BadRequestException (HTTP 400) when the name or the temp target is not valid */ - public void copyTo(CollectionResource collRes, String name) { + public void copyTo(CollectionResource collRes, String name) throws BadRequestException { + DotWebdavHelper.requirePlainSegment(this, name); User user=(User)HttpManager.request().getAuthorization().getTag(); if(collRes instanceof TempFolderResourceImpl){ TempFolderResourceImpl tr = (TempFolderResourceImpl)collRes; + final File dest = dotDavHelper.resolveTempChild(this, tr.getFolder(), name); try { - String p = tr.getFolder().getPath(); - File dest = new File(tr.getFolder().getPath() + File.separator + name); FileUtil.copyDirectory(folder, dest); } catch (Exception e) { Logger.error(this, e.getMessage(), e); @@ -237,15 +258,21 @@ public void sendContent(OutputStream arg0, Range arg1, Map arg2, return; } - /* (non-Javadoc) - * @see com.bradmcevoy.http.MoveableResource#moveTo(com.bradmcevoy.http.CollectionResource, java.lang.String) + /** + * Moves this temp folder into {@code collRes} under {@code name}. The name must be a single plain + * path segment, and a move into temp storage must stay inside the temp directory. + * + * @param collRes the destination collection + * @param name the destination name received from the WebDAV client + * @throws BadRequestException (HTTP 400) when the name or the temp target is not valid */ - public void moveTo(CollectionResource collRes, String name) { + public void moveTo(CollectionResource collRes, String name) throws BadRequestException { + DotWebdavHelper.requirePlainSegment(this, name); User user=(User)HttpManager.request().getAuthorization().getTag(); if(collRes instanceof TempFolderResourceImpl){ TempFolderResourceImpl tr = (TempFolderResourceImpl)collRes; + final File dest = dotDavHelper.resolveTempChild(this, tr.getFolder(), name); try { - File dest = new File(tr.getFolder().getPath() + File.separator + name); FileUtil.copyDirectory(folder, dest); FileUtil.deltree(folder, true); } catch (Exception e) { @@ -333,9 +360,23 @@ public Long getMaxAgeSeconds(Auth arg0) { return (long)60; } + /** + * Creates a sub-folder named {@code name} and locks it. The lock interface only allows + * {@link NotAuthorizedException}, so an invalid name is reported as a runtime error. + * + * @param name the folder name received from the WebDAV client + * @param timeout the requested lock timeout + * @param lockInfo the requested lock details + * @return the lock token of the created folder + * @throws NotAuthorizedException if the lock is not authorized + */ public LockToken createAndLock(String name, LockTimeout timeout, LockInfo lockInfo) throws NotAuthorizedException { - createCollection(name); + try { + createCollection(name); + } catch (BadRequestException e) { + throw new DotRuntimeException(e.getReason(), e); + } return lock(timeout, lockInfo).getLockToken(); } diff --git a/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java b/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java index bb896aee5643..cb6195c2d67a 100644 --- a/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java +++ b/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java @@ -139,7 +139,8 @@ SystemEventsReconciliationIntegrationTest.class, SystemEventsRetentionIntegrationTest.class, com.dotcms.rest.api.v1.portlet.PortletResourceIntegrationTest.class, - LayoutResourceIntegrationTest.class + LayoutResourceIntegrationTest.class, + com.dotmarketing.webdav.DotWebdavTempPathContainmentTest.class }) public class MainSuite3a { diff --git a/dotcms-integration/src/test/java/com/dotmarketing/webdav/DotWebdavTempPathContainmentTest.java b/dotcms-integration/src/test/java/com/dotmarketing/webdav/DotWebdavTempPathContainmentTest.java new file mode 100644 index 000000000000..d75981b1f40e --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotmarketing/webdav/DotWebdavTempPathContainmentTest.java @@ -0,0 +1,187 @@ +package com.dotmarketing.webdav; + +import com.bradmcevoy.http.Auth; +import com.bradmcevoy.http.HttpManager; +import com.bradmcevoy.http.Request; +import com.bradmcevoy.http.exceptions.BadRequestException; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.business.APILocator; +import com.liferay.util.FileUtil; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import org.junit.After; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +/** + * WebDAV temp storage must only read and write strictly inside the temp directory + * ({@link DotWebdavHelper#getTempDir()}), and a name that reaches temp storage must be a single plain + * path segment. Rejected names surface as {@link BadRequestException} (HTTP 400) and leave the + * filesystem untouched. In-bounds names, including the dot-prefixed transient files WebDAV clients + * create, keep working. + */ +public class DotWebdavTempPathContainmentTest { + + private static final byte[] CONTENT = "content".getBytes(StandardCharsets.UTF_8); + + private final DotWebdavHelper helper = new DotWebdavHelper(); + private final List scratchFolders = new ArrayList<>(); + + @BeforeClass + public static void prepare() throws Exception { + IntegrationTestInitService.getInstance().init(); + } + + @After + public void cleanUp() { + scratchFolders.forEach(folder -> FileUtil.deltree(folder, true)); + } + + /** Creates an empty, uniquely named folder inside the temp directory for one test. */ + private File newScratchFolder() { + final File folder = new File(helper.getTempDir(), "containment-" + UUID.randomUUID()); + Assert.assertTrue(folder.mkdirs()); + scratchFolders.add(folder); + return folder; + } + + private static boolean isStrictlyInside(final File root, final File candidate) throws IOException { + return candidate.getCanonicalPath().startsWith(root.getCanonicalPath() + File.separator); + } + + @Test + public void loadTempFile_returns_null_for_a_path_outside_the_temp_dir() { + Assert.assertNull(helper.loadTempFile("/webdav/working/1/../outside.txt")); + } + + @Test + public void loadTempFile_returns_null_for_the_temp_dir_itself() { + Assert.assertNull(helper.loadTempFile("/webdav/working/1/somehost/..")); + } + + @Test + public void loadTempFile_allows_an_in_bounds_dot_prefixed_temp_file() throws IOException { + final File resolved = helper.loadTempFile("/webdav/working/1/demo.dotcms.com/.inflight.txt"); + Assert.assertNotNull(resolved); + Assert.assertTrue(isStrictlyInside(helper.getTempDir(), resolved)); + } + + @Test + public void createTempFolder_rejects_a_path_outside_the_temp_dir() { + final File outside = new File(helper.getTempDir().getParentFile(), "outside-dir"); + try { + Assert.assertThrows(RuntimeException.class, + () -> helper.createTempFolder("/webdav/working/1/../outside-dir")); + Assert.assertFalse(outside.exists()); + } finally { + if (outside.exists()) { + FileUtil.deltree(outside, true); + } + } + } + + @Test + public void createTempFile_rejects_a_path_outside_the_temp_dir() { + final File outside = new File(helper.getTempDir().getParentFile(), "outside-file.txt"); + try { + Assert.assertThrows(IOException.class, () -> helper.createTempFile("/../outside-file.txt")); + Assert.assertFalse(outside.exists()); + } finally { + if (outside.exists()) { + Assert.assertTrue(outside.delete()); + } + } + } + + @Test + public void createNew_rejects_a_name_that_is_not_a_plain_segment() { + final File scratch = newScratchFolder(); + final File folder = new File(scratch, "folder"); + Assert.assertTrue(folder.mkdirs()); + final TempFolderResourceImpl resource = new TempFolderResourceImpl(folder.getPath(), folder, false); + + for (final String name : new String[]{"..", ".", "a\\b"}) { + Assert.assertThrows("name: " + name, BadRequestException.class, + () -> resource.createNew(name, new ByteArrayInputStream(CONTENT), + (long) CONTENT.length, "text/plain")); + } + Assert.assertArrayEquals(new String[0], folder.list()); + } + + @Test + public void createNew_writes_a_plain_name_inside_the_folder() throws Exception { + final File scratch = newScratchFolder(); + final TempFolderResourceImpl resource = new TempFolderResourceImpl(scratch.getPath(), scratch, false); + + resource.createNew("plain.txt", new ByteArrayInputStream(CONTENT), (long) CONTENT.length, "text/plain"); + + final File written = new File(scratch, "plain.txt"); + Assert.assertTrue(written.isFile()); + Assert.assertArrayEquals(CONTENT, Files.readAllBytes(written.toPath())); + } + + @Test + public void createCollection_rejects_a_name_that_is_not_a_plain_segment() { + final File scratch = newScratchFolder(); + final File folder = new File(scratch, "folder"); + Assert.assertTrue(folder.mkdirs()); + final TempFolderResourceImpl resource = new TempFolderResourceImpl(folder.getPath(), folder, false); + + for (final String name : new String[]{"..", "."}) { + Assert.assertThrows("name: " + name, BadRequestException.class, + () -> resource.createCollection(name)); + } + } + + @Test + public void copyTo_rejects_a_name_that_is_not_a_plain_segment() throws Exception { + assertCopyOrMoveRejected(true); + } + + @Test + public void moveTo_rejects_a_name_that_is_not_a_plain_segment() throws Exception { + assertCopyOrMoveRejected(false); + } + + private void assertCopyOrMoveRejected(final boolean copy) throws Exception { + final File scratch = newScratchFolder(); + final File sourceFolder = new File(scratch, "source"); + final File targetFolder = new File(new File(scratch, "target"), "inner"); + Assert.assertTrue(sourceFolder.mkdirs()); + Assert.assertTrue(targetFolder.mkdirs()); + final File sourceFile = new File(sourceFolder, "file.txt"); + Files.write(sourceFile.toPath(), CONTENT); + + final TempFileResourceImpl source = new TempFileResourceImpl(sourceFile, sourceFile.getPath(), false); + final TempFolderResourceImpl target = new TempFolderResourceImpl(targetFolder.getPath(), targetFolder, false); + + try (MockedStatic http = Mockito.mockStatic(HttpManager.class)) { + final Request request = Mockito.mock(Request.class); + final Auth auth = Mockito.mock(Auth.class); + Mockito.when(auth.getTag()).thenReturn(APILocator.getUserAPI().getSystemUser()); + Mockito.when(request.getAuthorization()).thenReturn(auth); + http.when(HttpManager::request).thenReturn(request); + + Assert.assertThrows(BadRequestException.class, () -> { + if (copy) { + source.copyTo(target, ".."); + } else { + source.moveTo(target, ".."); + } + }); + } + + Assert.assertTrue(sourceFile.isFile()); + Assert.assertTrue(targetFolder.isDirectory()); + Assert.assertArrayEquals(new String[0], targetFolder.list()); + } +}