You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by ha...@apache.org on 2026/08/14 11:04:16 UTC
(knox) branch v3.0.0 updated: KNOX-3418: Path traversal → arbitrary file write/overwrite in the Apache Knox Admin API (#1349)
This is an automated email from the ASF dual-hosted git repository.
hanicz pushed a commit to branch v3.0.0
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/v3.0.0 by this push:
new cab2606b9 KNOX-3418: Path traversal → arbitrary file write/overwrite in the Apache Knox Admin API (#1349)
cab2606b9 is described below
commit cab2606b97955d321638b7e65249da992d9572fb
Author: hanicz <ha...@users.noreply.github.com>
AuthorDate: Fri Aug 14 13:02:59 2026 +0200
KNOX-3418: Path traversal → arbitrary file write/overwrite in the Apache Knox Admin API (#1349)
(cherry picked from commit de71a60cdf34c842abb8a49f770a0e33fe33c7e9)
---
.../topology/impl/DefaultTopologyService.java | 17 +++-
.../ZkRemoteConfigurationMonitorService.java | 4 +-
.../topology/DefaultTopologyServiceTest.java | 39 ++++++++
.../gateway/service/admin/TopologiesResource.java | 37 +++----
.../service/admin/TopologyResourceTest.java | 108 +++++++++++++++++++++
.../knox/gateway/i18n/GatewaySpiMessages.java | 3 +
.../knox/gateway/GatewayAdminTopologyFuncTest.java | 3 +-
7 files changed, 183 insertions(+), 28 deletions(-)
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
index 02d916f29..0468e4aee 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java
@@ -306,13 +306,17 @@ public class DefaultTopologyService extends FileAlterationListenerAdaptor implem
public void deployTopology(Topology t){
try {
+ File topology = new File(topologiesDirectory.getAbsolutePath() + "/" + t.getName() + ".xml");
+ if (!topology.getCanonicalFile().toPath().startsWith(topologiesDirectory.getCanonicalFile().toPath())) {
+ throw new IOException("Resolved topology path escapes managed directory: " + t.getName());
+ }
+
File temp = new File(topologiesDirectory.getAbsolutePath() + "/" + t.getName() + ".xml.temp");
Marshaller mr = jaxbContext.createMarshaller();
mr.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, true);
mr.marshal(t, temp);
- File topology = new File(topologiesDirectory.getAbsolutePath() + "/" + t.getName() + ".xml");
if(!temp.renameTo(topology)) {
FileUtils.forceDelete(temp);
throw new IOException("Could not rename temp file");
@@ -716,8 +720,15 @@ public class DefaultTopologyService extends FileAlterationListenerAdaptor implem
File destFile = new File(dest, name);
try {
- FileUtils.writeStringToFile(destFile, content, StandardCharsets.UTF_8);
- log.wroteConfigurationFile(destFile.getAbsolutePath());
+ final File canonicalDest = dest.getCanonicalFile();
+ final File canonicalFile = destFile.getCanonicalFile();
+ if (!canonicalFile.toPath().startsWith(canonicalDest.toPath())) {
+ log.failedToWriteConfigurationFile(destFile.getAbsolutePath(),
+ new IOException("Resolved path escapes managed directory: " + name));
+ return false;
+ }
+ FileUtils.writeStringToFile(canonicalFile, content, StandardCharsets.UTF_8);
+ log.wroteConfigurationFile(canonicalFile.getAbsolutePath());
result = true;
} catch (IOException e) {
log.failedToWriteConfigurationFile(destFile.getAbsolutePath(), e);
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/ZkRemoteConfigurationMonitorService.java b/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/ZkRemoteConfigurationMonitorService.java
index 0d8e34c57..2c6d89a38 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/ZkRemoteConfigurationMonitorService.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/ZkRemoteConfigurationMonitorService.java
@@ -212,14 +212,14 @@ class ZkRemoteConfigurationMonitorService implements RemoteConfigurationMonitor
@Override
public boolean createProvider(String name, String content) {
- String entryPath = "/knox/config/shared-providers/" + name;
+ String entryPath = "/knox/config/shared-providers/" + FilenameUtils.getName(name);
client.createEntry(entryPath, content);
return (client.getEntryData(entryPath) != null);
}
@Override
public boolean createDescriptor(String name, String content) {
- String entryPath = "/knox/config/descriptors/" + name;
+ String entryPath = "/knox/config/descriptors/" + FilenameUtils.getName(name);
client.createEntry(entryPath, content);
return (client.getEntryData(entryPath) != null);
}
diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java
index bf7cf976c..e428d1e84 100644
--- a/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java
+++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java
@@ -595,6 +595,45 @@ public class DefaultTopologyServiceTest {
}
}
+ @Test
+ public void testDeployRejectsPathTraversal() throws Exception {
+ File dir = createDir();
+ File topologyDir = new File(dir, "topologies");
+ topologyDir.mkdirs();
+
+ File descriptorsDir = new File(dir, "descriptors");
+ descriptorsDir.mkdirs();
+
+ File sharedProvidersDir = new File(dir, "shared-providers");
+ sharedProvidersDir.mkdirs();
+
+ try {
+ TopologyService ts = new DefaultTopologyService();
+ Map<String, String> c = new HashMap<>();
+
+ GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+ EasyMock.expect(config.getReadOnlyOverrideTopologyNames()).andReturn(Collections.emptyList()).anyTimes();
+ EasyMock.expect(config.getGatewayTopologyDir()).andReturn(topologyDir.getAbsolutePath()).anyTimes();
+ EasyMock.expect(config.getGatewayConfDir()).andReturn(descriptorsDir.getParentFile().getAbsolutePath()).anyTimes();
+ EasyMock.replay(config);
+
+ ts.init(config, c);
+
+ final String traversalName = "../../evil.json";
+ final File escapeTarget = new File(dir.getParentFile(), "evil.json");
+ assertFalse("precondition: escape target must not pre-exist", escapeTarget.exists());
+
+ assertFalse("deployProviderConfiguration must reject a path-traversal name",
+ ts.deployProviderConfiguration(traversalName, "malicious"));
+ assertFalse("deployDescriptor must reject a path-traversal name",
+ ts.deployDescriptor(traversalName, "malicious"));
+
+ assertFalse("no file may be written outside the managed directory", escapeTarget.exists());
+ } finally {
+ FileUtils.deleteQuietly(dir);
+ }
+ }
+
@Test
public void testProviderParamsOrderIsPreserved() {
diff --git a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java
index 2c66faafd..98279a579 100644
--- a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java
+++ b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java
@@ -43,6 +43,7 @@ import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
@@ -55,8 +56,6 @@ import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
@@ -90,7 +89,7 @@ public class TopologiesResource {
private static final String SINGLE_DESCRIPTOR_API_PATH = DESCRIPTORS_API_PATH + "/{name}";
private static final int RESOURCE_NAME_LENGTH_MAX = 100;
- private static final Pattern RESOURCE_NAME_PATTERN = Pattern.compile("^[\\w-/.]+$");
+ private static final Pattern RESOURCE_NAME_PATTERN = Pattern.compile("^[\\w.-]+$");
private static GatewaySpiMessages log = MessagesFactory.get(GatewaySpiMessages.class);
@@ -171,12 +170,6 @@ public class TopologiesResource {
public Topology uploadTopology(@PathParam("id") String id, Topology t) {
Topology result = null;
- try {
- id = URLDecoder.decode(id, StandardCharsets.UTF_8.name());
- } catch (Exception e) {
- // Ignore
- }
-
if (!isValidResourceName(id)) {
log.invalidResourceName(id);
throw new BadRequestException("Invalid topology name: " + id);
@@ -188,6 +181,17 @@ public class TopologiesResource {
t.setName(id);
TopologyService ts = gs.getService(ServiceType.TOPOLOGY_SERVICE);
+ GatewayConfig config =
+ (GatewayConfig) request.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ if (config != null &&
+ config.getReadOnlyOverrideTopologyNames().contains(FilenameUtils.getBaseName(id))) {
+ log.disallowedOverwritingReadOnlyTopology(id);
+ throw new WebApplicationException(
+ status(Response.Status.FORBIDDEN)
+ .entity("{ \"error\" : \"Cannot overwrite read-only topology: " + id + "\" }")
+ .build());
+ }
+
// Check for existing topology with the same name, to see if it had been generated
boolean existingGenerated = false;
for (org.apache.knox.gateway.topology.Topology existingTopology : ts.getTopologies()) {
@@ -338,12 +342,6 @@ public class TopologiesResource {
public Response uploadProviderConfiguration(@PathParam("name") String name, @Context HttpHeaders headers, String content) {
Response response = null;
- try {
- name = URLDecoder.decode(name, StandardCharsets.UTF_8.name());
- } catch (Exception e) {
- // Ignore
- }
-
if (!isValidResourceName(name)) {
log.invalidResourceName(name);
throw new BadRequestException("Invalid provider configuration name: " + name);
@@ -393,12 +391,6 @@ public class TopologiesResource {
String content) {
Response response = null;
- try {
- name = URLDecoder.decode(name, StandardCharsets.UTF_8.name());
- } catch (Exception e) {
- // Ignore
- }
-
if (!isValidResourceName(name)) {
log.invalidResourceName(name);
throw new BadRequestException("Invalid descriptor name: " + name);
@@ -554,8 +546,9 @@ public class TopologiesResource {
return result;
}
- private static boolean isValidResourceName(final String name) {
+ static boolean isValidResourceName(final String name) {
return name != null && name.length() <= RESOURCE_NAME_LENGTH_MAX &&
+ !name.contains("..") &&
RESOURCE_NAME_PATTERN.matcher(name).matches();
}
diff --git a/gateway-service-admin/src/test/java/org/apache/knox/gateway/service/admin/TopologyResourceTest.java b/gateway-service-admin/src/test/java/org/apache/knox/gateway/service/admin/TopologyResourceTest.java
index e49015e1f..d0e225ed1 100644
--- a/gateway-service-admin/src/test/java/org/apache/knox/gateway/service/admin/TopologyResourceTest.java
+++ b/gateway-service-admin/src/test/java/org/apache/knox/gateway/service/admin/TopologyResourceTest.java
@@ -19,7 +19,18 @@ package org.apache.knox.gateway.service.admin;
import org.apache.knox.gateway.topology.Topology;
import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.topology.TopologyService;
+
+import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import java.lang.reflect.Field;
+import java.util.Collections;
+import java.util.List;
import org.easymock.EasyMock;
import org.junit.Test;
@@ -27,6 +38,9 @@ import org.junit.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
public class TopologyResourceTest {
@@ -174,6 +188,28 @@ public class TopologyResourceTest {
}
+ @Test
+ public void testResourceNameValidation() {
+ assertTrue(TopologiesResource.isValidResourceName("foo"));
+ assertTrue(TopologiesResource.isValidResourceName("foo.json"));
+ assertTrue(TopologiesResource.isValidResourceName("my-provider_1"));
+ assertTrue(TopologiesResource.isValidResourceName("a.b.c"));
+
+ assertFalse(TopologiesResource.isValidResourceName("../../etc/passwd"));
+ assertFalse(TopologiesResource.isValidResourceName(".."));
+ assertFalse(TopologiesResource.isValidResourceName("foo/bar"));
+ assertFalse(TopologiesResource.isValidResourceName("a/../../b"));
+ assertFalse(TopologiesResource.isValidResourceName("foo..bar"));
+
+ assertFalse(TopologiesResource.isValidResourceName("%2f"));
+ assertFalse(TopologiesResource.isValidResourceName("%252f"));
+
+ assertFalse(TopologiesResource.isValidResourceName(null));
+ assertFalse(TopologiesResource.isValidResourceName(""));
+ assertFalse(TopologiesResource.isValidResourceName("a".repeat(101)));
+ assertTrue(TopologiesResource.isValidResourceName("a".repeat(100)));
+ }
+
private void setDefaultExpectations(HttpServletRequest request){
EasyMock.expect( request.getPathInfo() ).andReturn( pathInfo ).anyTimes();
EasyMock.expect( request.getContextPath() ).andReturn( reqContext ).anyTimes();
@@ -186,4 +222,76 @@ public class TopologyResourceTest {
EasyMock.expect( request.getHeader( header ) ).andReturn( expected ).anyTimes();
}
+ @Test
+ public void testUploadTopologyRefusesReadOnlyOverride() throws Exception {
+ TopologyService ts = EasyMock.createMock(TopologyService.class);
+
+ GatewayServices gs = EasyMock.createNiceMock(GatewayServices.class);
+ EasyMock.expect(gs.getService(ServiceType.TOPOLOGY_SERVICE)).andReturn(ts).anyTimes();
+
+ GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+ EasyMock.expect(config.getReadOnlyOverrideTopologyNames())
+ .andReturn(List.of("manager")).anyTimes();
+
+ HttpServletRequest request = mockRequest(gs, config);
+
+ EasyMock.replay(ts, gs, config, request);
+
+ TopologiesResource res = new TopologiesResource();
+ setRequestField(res, request);
+
+ try {
+ res.uploadTopology("manager", new org.apache.knox.gateway.service.admin.beans.Topology());
+ fail("Expected WebApplicationException for read-only override topology");
+ } catch (WebApplicationException e) {
+ assertEquals(Response.Status.FORBIDDEN.getStatusCode(), e.getResponse().getStatus());
+ }
+
+ EasyMock.verify(ts);
+ }
+
+ @Test
+ public void testUploadTopologyAllowedWhenNotReadOnly() throws Exception {
+ TopologyService ts = EasyMock.createMock(TopologyService.class);
+ EasyMock.expect(ts.getTopologies()).andReturn(Collections.emptyList()).anyTimes();
+ ts.deployTopology(EasyMock.anyObject(Topology.class));
+ EasyMock.expectLastCall().once();
+
+ GatewayServices gs = EasyMock.createNiceMock(GatewayServices.class);
+ EasyMock.expect(gs.getService(ServiceType.TOPOLOGY_SERVICE)).andReturn(ts).anyTimes();
+
+ GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+ EasyMock.expect(config.getReadOnlyOverrideTopologyNames())
+ .andReturn(Collections.emptyList()).anyTimes();
+
+ HttpServletRequest request = mockRequest(gs, config);
+
+ EasyMock.replay(ts, gs, config, request);
+
+ TopologiesResource res = new TopologiesResource();
+ setRequestField(res, request);
+
+ res.uploadTopology("sandbox", new org.apache.knox.gateway.service.admin.beans.Topology());
+
+ EasyMock.verify(ts);
+ }
+
+ private HttpServletRequest mockRequest(GatewayServices gs, GatewayConfig config) {
+ ServletContext context = EasyMock.createNiceMock(ServletContext.class);
+ EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE))
+ .andReturn(gs).anyTimes();
+ EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE))
+ .andReturn(config).anyTimes();
+ HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(request.getServletContext()).andReturn(context).anyTimes();
+ EasyMock.replay(context);
+ return request;
+ }
+
+ private void setRequestField(TopologiesResource res, HttpServletRequest request) throws Exception {
+ Field f = TopologiesResource.class.getDeclaredField("request");
+ f.setAccessible(true);
+ f.set(res, request);
+ }
+
}
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java b/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java
index 721749970..a067af3b5 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java
@@ -76,6 +76,9 @@ public interface GatewaySpiMessages {
@Message( level = MessageLevel.ERROR, text = "Topology {0} cannot be manually overwritten because it was generated from a simple descriptor." )
void disallowedOverwritingGeneratedTopology(String topologyName);
+ @Message( level = MessageLevel.INFO, text = "Read-only topology {0} cannot be overwritten." )
+ void disallowedOverwritingReadOnlyTopology(String topologyName);
+
@Message( level = MessageLevel.INFO, text = "Read-only descriptor {0} cannot be overwritten." )
void disallowedOverwritingGeneratedDescriptor(String name);
diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
index 4928ed920..417ab5949 100644
--- a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
+++ b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
@@ -1806,7 +1806,8 @@ public class GatewayAdminTopologyFuncTest {
String newDescriptorJSON = createDescriptor(clusterName);
// Attempt to PUT the descriptor
- given().auth().preemptive().basic(username, password)
+ given().urlEncodingEnabled(false)
+ .auth().preemptive().basic(username, password)
.header("Content-type", MediaType.APPLICATION_JSON)
.body(newDescriptorJSON.getBytes(StandardCharsets.UTF_8.name()))
.then()