Skip to content

Prevent path traversal in the asset serving REST resources (7.0) #26987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
graylog-internal-actions-access wants to merge 1 commit into
base: 7.0
Choose a base branch
from
Draft

Prevent path traversal in the asset serving REST resources (7.0) #26987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/unreleased/ghsa-7v9x-j5xj-57rq.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type = "s"
message = "Fix path traversal in the web interface and preflight asset resources"
details.users = "See [GHSA-7v9x-j5xj-57rq](https://github.com/Graylog2/graylog2-server/security/advisories/GHSA-7v9x-j5xj-57rq) for details."
pulls = ["26942"]
2 changes: 2 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/bootstrap/preflight/PreflightWebModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.graylog2.shared.bindings.providers.EventBusProvider;
import org.graylog2.shared.bindings.providers.ServiceManagerProvider;
import org.graylog2.shared.initializers.PeriodicalsService;
import org.graylog2.web.resources.ResourceFileReader;

import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -75,6 +76,7 @@ protected void configure() {
bind(PreflightConfigService.class).to(PreflightConfigServiceImpl.class);
bind(PreflightBoot.class).asEagerSingleton();
bind(NotificationService.class).to(NullNotificationService.class);
bind(ResourceFileReader.class).asEagerSingleton();

addPreflightRestResource(PreflightResource.class);
addPreflightRestResource(CertificateRenewalPolicyResource.class);
Expand Down
95 changes: 20 additions & 75 deletions ...src/main/java/org/graylog2/bootstrap/preflight/web/resources/PreflightAssetsResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,7 @@
*/
package org.graylog2.bootstrap.preflight.web.resources;

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.hash.HashCode;
import com.google.common.hash.Hashing;
import com.google.common.io.Resources;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.graylog2.bootstrap.preflight.PreflightConstants;

import javax.activation.MimetypesFileTypeMap;
import javax.annotation.Nonnull;

import jakarta.inject.Inject;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.NotFoundException;
import jakarta.ws.rs.Path;
Expand All @@ -41,50 +28,32 @@
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Request;
import jakarta.ws.rs.core.Response;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.graylog2.bootstrap.preflight.PreflightConstants;
import org.graylog2.bootstrap.preflight.PreflightWebModule;
import org.graylog2.web.resources.ResourceFileReader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.FileNotFoundException;
import javax.activation.MimetypesFileTypeMap;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.FileSystem;
import java.nio.file.FileSystemAlreadyExistsException;
import java.nio.file.FileSystemNotFoundException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.nio.file.attribute.FileTime;
import java.util.Collections;
import java.util.Date;
import java.util.concurrent.TimeUnit;

import static com.google.common.base.MoreObjects.firstNonNull;

@Path("/")
public class PreflightAssetsResource {
private static final Logger LOG = LoggerFactory.getLogger(PreflightAssetsResource.class);

private final MimetypesFileTypeMap mimeTypes;
private final LoadingCache<URI, FileSystem> fileSystemCache;
private final ResourceFileReader resourceFileReader;

@Inject
public PreflightAssetsResource(MimetypesFileTypeMap mimeTypes) {
public PreflightAssetsResource(MimetypesFileTypeMap mimeTypes, ResourceFileReader resourceFileReader) {
this.mimeTypes = mimeTypes;
this.fileSystemCache = CacheBuilder.newBuilder()
.maximumSize(1024)
.build(new CacheLoader<URI, FileSystem>() {
@Override
public FileSystem load(@Nonnull URI key) throws Exception {
try {
return FileSystems.getFileSystem(key);
} catch (FileSystemNotFoundException e) {
try {
return FileSystems.newFileSystem(key, Collections.emptyMap());
} catch (FileSystemAlreadyExistsException f) {
return FileSystems.getFileSystem(key);
}
}
}
});
this.resourceFileReader = resourceFileReader;
}

@Produces(MediaType.TEXT_HTML)
Expand All @@ -99,43 +68,19 @@ public Response index(@Context Request request) {
@RequiresPermissions(PreflightWebModule.PERMISSION_PREFLIGHT_ONLY)
public Response get(@Context Request request, @PathParam("filename") String filename) {
try {
final URL resourceUrl = getResourceUri(filename);
return getResponse(request, filename, resourceUrl);
final var resource = resourceFileReader.readFileFrom(PreflightConstants.ASSETS_RESOURCE_DIR, filename, getClass());
return getResponse(request, filename, resource);
} catch (IOException | URISyntaxException e) {
throw new NotFoundException("Couldn't find " + filename, e);
}
}

private URL getResourceUri(String filename) throws FileNotFoundException {
final URL resourceUrl = this.getClass().getResource(PreflightConstants.ASSETS_RESOURCE_DIR + filename);
if (resourceUrl == null) {
throw new FileNotFoundException("Resource file " + filename + " not found.");
LOG.debug("Couldn't serve preflight asset <{}>.", filename, e);
// Don't reflect the requested file name back to the client.
throw new NotFoundException("Couldn't find the requested resource.", e);
}
return resourceUrl;
}

private Response getResponse(Request request, String filename, URL resourceUrl) throws IOException, URISyntaxException {
final URI uri = resourceUrl.toURI();

final java.nio.file.Path path;
final byte[] fileContents;
switch (resourceUrl.getProtocol()) {
case "file" -> {
path = Paths.get(uri);
fileContents = Files.readAllBytes(path);
}
case "jar" -> {
final FileSystem fileSystem = fileSystemCache.getUnchecked(uri);
path = fileSystem.getPath(PreflightConstants.ASSETS_RESOURCE_DIR + filename);
fileContents = Resources.toByteArray(resourceUrl);
}
default -> throw new IllegalArgumentException("Not a JAR or local file: " + resourceUrl);
}

final FileTime lastModifiedTime = Files.getLastModifiedTime(path);
final Date lastModified = Date.from(lastModifiedTime.toInstant());
final HashCode hashCode = Hashing.sha256().hashBytes(fileContents);
final EntityTag entityTag = new EntityTag(hashCode.toString());
private Response getResponse(Request request, String filename, ResourceFileReader.ResourceFile resource) {
final byte[] fileContents = resource.contents().get();
final Date lastModified = resource.lastModified().orElseGet(Date::new);
final EntityTag entityTag = resource.entityTag().get();

final Response.ResponseBuilder response = request.evaluatePreconditions(lastModified, entityTag);
if (response != null) {
Expand Down
124 changes: 111 additions & 13 deletions graylog2-server/src/main/java/org/graylog2/web/resources/ResourceFileReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.graylog2.web.resources;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Suppliers;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
Expand All @@ -28,6 +29,7 @@
import org.graylog2.web.PluginAssets;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URI;
Expand All @@ -38,15 +40,23 @@
import java.nio.file.FileSystemNotFoundException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Paths;
import java.nio.file.attribute.FileTime;
import java.security.CodeSource;
import java.util.Collections;
import java.util.Date;
import java.util.Optional;
import java.util.function.Supplier;

@Singleton
public class ResourceFileReader {
/**
* Plugin assets are packaged at the root of the plugin JAR.
*/
public static final String PLUGIN_ASSETS_DIR = "/";
public static final String WEB_INTERFACE_ASSETS_DIR = "/" + PluginAssets.pathPrefix + "/";

private final LoadingCache<URI, FileSystem> fileSystemCache;

@Inject
Expand Down Expand Up @@ -93,17 +103,32 @@ public Optional<Date> lastModified() {
}

public ResourceFile readFile(String filename, Class<?> aClass) throws URISyntaxException, IOException {
return readFile(false, filename, aClass);
return readFileFrom(WEB_INTERFACE_ASSETS_DIR, filename, aClass);
}

public ResourceFile readFileFromPlugin(String filename, Class<?> aClass) throws URISyntaxException, IOException {
return readFile(true, filename, aClass);
/**
* Reads an asset of the plugin that {@code pluginClass} belongs to.
* <p>
* Plugin assets are packaged at the root of the plugin JAR, which is also where every other JAR on the
* classpath keeps its own root-level resources. A classpath lookup delegates beyond the plugin, and a
* traversal sequence normalizes away at the root ({@code "/../../log4j2.xml"} becomes
* {@code "/log4j2.xml"}), so {@link #resolveResourceName(String, String)} alone cannot keep this from
* addressing an unrelated JAR. We therefore additionally require the resource to originate from the
* plugin's own code source.
*/
public ResourceFile readFileFromPlugin(String filename, Class<?> pluginClass) throws URISyntaxException, IOException {
final String resourceName = resolveResourceName(PLUGIN_ASSETS_DIR, filename);
if (!isFromCodeSourceOf(pluginClass.getResource(resourceName), pluginClass)) {
throw new FileNotFoundException("Resource file not found.");
}
return readFileFrom(PLUGIN_ASSETS_DIR, filename, pluginClass);
}

private ResourceFile readFile(boolean fromPlugin, String filename, Class<?> aClass) throws URISyntaxException, IOException {
final URL resourceUrl = aClass.getResource(pluginPrefixFilename(fromPlugin, filename));
public ResourceFile readFileFrom(String resourceDir, String filename, Class<?> aClass) throws URISyntaxException, IOException {
final String resourceName = resolveResourceName(resourceDir, filename);
final URL resourceUrl = aClass.getResource(resourceName);
if (resourceUrl == null) {
throw new FileNotFoundException("Resource file " + filename + " not found.");
throw new FileNotFoundException("Resource file not found.");
}
final URI uri = resourceUrl.toURI();

Expand All @@ -115,7 +140,7 @@ private ResourceFile readFile(boolean fromPlugin, String filename, Class<?> aCla
}
case "jar": {
final FileSystem fileSystem = fileSystemCache.getUnchecked(uri);
final java.nio.file.Path path = fileSystem.getPath(pluginPrefixFilename(fromPlugin, filename));
final java.nio.file.Path path = fileSystem.getPath(resourceName);
final var contents = Resources.toByteArray(resourceUrl);
return ResourceFile.create(path, contents);
}
Expand All @@ -124,13 +149,86 @@ private ResourceFile readFile(boolean fromPlugin, String filename, Class<?> aCla
}
}


/**
* Resolves {@code filename} inside {@code resourceDir} and verifies that the result is a direct child of
* it, so a path traversal sequence in the (already URL-decoded) filename cannot address resources outside
* of the directory.
* <p>
* The check requires a direct child rather than mere containment below {@code resourceDir}, because all
* asset directories we serve from are flat, and because containment is meaningless for
* {@link #PLUGIN_ASSETS_DIR}: {@code "/"} contains every path, including a traversed one.
*
* @return the canonical classpath resource name of the requested file
* @throws FileNotFoundException if the filename does not address a direct child of {@code resourceDir}
*/
@VisibleForTesting
@Nonnull
private String pluginPrefixFilename(boolean fromPlugin, String filename) {
if (fromPlugin) {
return "/" + filename;
} else {
return "/" + PluginAssets.pathPrefix + "/" + filename;
static String resolveResourceName(String resourceDir, String filename) throws FileNotFoundException {
final java.nio.file.Path base = Paths.get(resourceDir).normalize();
final java.nio.file.Path resolved;
try {
resolved = base.resolve(filename).normalize();
} catch (InvalidPathException e) {
throw new FileNotFoundException("Invalid resource file name.");
}
if (!base.equals(resolved.getParent())) {
throw new FileNotFoundException("Invalid resource file name.");
}
// Rebuild the name from the validated single path segment: classpath resource names always use "/"
// as a separator, whereas Path#toString() would use the platform-dependent one.
return resourceDir.endsWith("/")
? resourceDir + resolved.getFileName()
: resourceDir + "/" + resolved.getFileName();
}

/**
* Checks whether {@code resourceUrl} was loaded from the same JAR or classes directory that
* {@code aClass} itself came from.
*/
@VisibleForTesting
static boolean isFromCodeSourceOf(@Nullable URL resourceUrl, Class<?> aClass) {
if (resourceUrl == null) {
return false;
}
final CodeSource codeSource = aClass.getProtectionDomain().getCodeSource();
if (codeSource == null || codeSource.getLocation() == null) {
// Without a code source there is no way to tell where the resource came from, so refuse to
// serve it rather than falling back to the whole classpath.
return false;
}

// Compare local paths rather than URL strings, so that the two sides cannot differ merely in how they
// percent-encode a location, for example an installation directory containing a space.
final var codeSourcePath = toLocalPath(codeSource.getLocation().toString());
final var resourcePath = toLocalPath("jar".equals(resourceUrl.getProtocol())
? jarLocation(resourceUrl)
: resourceUrl.toString());
if (codeSourcePath.isEmpty() || resourcePath.isEmpty()) {
return false;
}

// A resource in a JAR resolves to the JAR itself, one on an exploded classpath to a file below the
// classes directory. Path#startsWith matches whole path segments, so a sibling directory with a
// common name prefix does not pass.
return resourcePath.get().startsWith(codeSourcePath.get());
}

private static Optional<java.nio.file.Path> toLocalPath(String url) {
try {
return Optional.of(Paths.get(URI.create(url)));
} catch (IllegalArgumentException | FileSystemNotFoundException e) {
return Optional.empty();
}
}

/**
* Strips the entry from a {@code jar:} URL, turning
* {@code jar:file:/opt/graylog/plugin/example.jar!/abc.js} into
* {@code file:/opt/graylog/plugin/example.jar}.
*/
private static String jarLocation(URL resourceUrl) {
final String url = resourceUrl.toString();
final int entrySeparator = url.indexOf("!/");
return entrySeparator < 0 ? url : url.substring("jar:".length(), entrySeparator);
}
}
8 changes: 7 additions & 1 deletion graylog2-server/src/main/java/org/graylog2/web/resources/WebInterfaceAssetsResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import org.graylog2.shared.rest.resources.csp.CSPDynamicFeature;
import org.graylog2.web.IndexHtmlGenerator;
import org.graylog2.web.customization.CustomizationConfig;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.activation.MimetypesFileTypeMap;
import java.io.IOException;
Expand All @@ -55,6 +57,8 @@
@CSP(group = CSP.DEFAULT)
@NonApiResource
public class WebInterfaceAssetsResource {
private static final Logger LOG = LoggerFactory.getLogger(WebInterfaceAssetsResource.class);

private static final String ASSETS_PREFIX = "assets";
private static final String FAVICON = "favicon.png";
private final MimetypesFileTypeMap mimeTypes;
Expand Down Expand Up @@ -122,7 +126,9 @@ public Response get(@Context Request request,
final var resource = resourceFileReader.readFileFromPlugin(filenameWithoutSuffix, plugin.metadata().getClass());
return getResponse(request, filenameWithoutSuffix, resource);
} catch (URISyntaxException | IOException e) {
throw new NotFoundException("Couldn't find " + filenameWithoutSuffix + " in plugin " + pluginName, e);
LOG.debug("Couldn't serve asset <{}> of plugin <{}>.", filenameWithoutSuffix, pluginName, e);
// Don't reflect the requested file name back to the client.
throw new NotFoundException("Couldn't find the requested resource in plugin " + pluginName, e);
}
}

Expand Down
Loading
Loading