You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2019/01/11 10:34:00 UTC
[camel] 01/03: CAMEL-13042: File producer should by default only allow to write file… (#2700)
This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch camel-2.21.x
in repository https://gitbox.apache.org/repos/asf/camel.git
commit 5e1d70c6957703cdebbfe5d796462e5a89c8bc26
Author: Claus Ibsen <cl...@gmail.com>
AuthorDate: Wed Jan 9 10:37:33 2019 +0100
CAMEL-13042: File producer should by default only allow to write file… (#2700)
* CAMEL-13042: File producer should by default only allow to write files in the starting directory (or subs). Added new option to turn this on|off.
* CAMEL-13042: Regen docs
* CAMEL-13042: Polished
---
.../camel/component/file/GenericFileEndpoint.java | 15 +++++
.../camel/component/file/GenericFileProducer.java | 12 +++-
.../component/file/FileProducerExpressionTest.java | 2 +-
.../FileProducerJailStartingDirectoryTest.java | 73 ++++++++++++++++++++++
.../file/remote/FtpProducerExpressionTest.java | 2 +-
.../FtpProducerJailStartingDirectoryTest.java | 68 ++++++++++++++++++++
6 files changed, 169 insertions(+), 3 deletions(-)
diff --git a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java
index def2f60..dc96ece 100644
--- a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java
+++ b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java
@@ -91,6 +91,8 @@ public abstract class GenericFileEndpoint<T> extends ScheduledPollEndpoint imple
protected boolean keepLastModified;
@UriParam(label = "producer,advanced")
protected boolean allowNullBody;
+ @UriParam(label = "producer", defaultValue = "true")
+ protected boolean jailStartingDirectory = true;
// consumer options
@@ -1178,6 +1180,19 @@ public abstract class GenericFileEndpoint<T> extends ScheduledPollEndpoint imple
this.allowNullBody = allowNullBody;
}
+ public boolean isJailStartingDirectory() {
+ return jailStartingDirectory;
+ }
+
+ /**
+ * Used for jailing (restricting) writing files to the starting directory (and sub) only.
+ * This is enabled by default to not allow Camel to write files to outside directories (to be more secured out of the box).
+ * You can turn this off to allow writing files to directories outside the starting directory, such as parent or root folders.
+ */
+ public void setJailStartingDirectory(boolean jailStartingDirectory) {
+ this.jailStartingDirectory = jailStartingDirectory;
+ }
+
public ExceptionHandler getOnCompletionExceptionHandler() {
return onCompletionExceptionHandler;
}
diff --git a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java
index 2dec738..7cd2b66 100644
--- a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java
+++ b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java
@@ -20,6 +20,7 @@ import java.io.File;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import org.apache.camel.CamelExchangeException;
import org.apache.camel.Exchange;
import org.apache.camel.Expression;
import org.apache.camel.impl.DefaultExchange;
@@ -331,7 +332,7 @@ public class GenericFileProducer<T> extends DefaultProducer {
exchange.getIn().setHeader(Exchange.FILE_NAME, value);
}
- if (value != null && value instanceof String && StringHelper.hasStartToken((String) value, "simple")) {
+ if (value instanceof String && StringHelper.hasStartToken((String) value, "simple")) {
log.warn("Simple expression: {} detected in header: {} of type String. This feature has been removed (see CAMEL-6748).", value, Exchange.FILE_NAME);
}
@@ -378,6 +379,15 @@ public class GenericFileProducer<T> extends DefaultProducer {
answer = baseDir + endpoint.getGeneratedFileName(exchange.getIn());
}
+ if (endpoint.isJailStartingDirectory()) {
+ // check for file must be within starting directory (need to compact first as the name can be using relative paths via ../ etc)
+ String compatchAnswer = FileUtil.compactPath(answer);
+ String compatchBaseDir = FileUtil.compactPath(baseDir);
+ if (!compatchAnswer.startsWith(compatchBaseDir)) {
+ throw new IllegalArgumentException("Cannot write file with name: " + compatchAnswer + " as the filename is jailed to the starting directory: " + compatchBaseDir);
+ }
+ }
+
if (endpoint.getConfiguration().needToNormalize()) {
// must normalize path to cater for Windows and other OS
answer = normalizePath(answer);
diff --git a/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java
index ac7b8d7..b56f419 100644
--- a/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java
+++ b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java
@@ -74,7 +74,7 @@ public class FileProducerExpressionTest extends ContextTestSupport {
public void testProducerComplexByExpression() throws Exception {
String expression = "../filelanguageinbox/myfile-${bean:myguidgenerator.guid}-${date:now:yyyyMMdd}.txt";
- template.sendBody("file://target/filelanguage?fileName=" + expression, "Hello World");
+ template.sendBody("file://target/filelanguage?jailStartingDirectory=false&fileName=" + expression, "Hello World");
String date = new SimpleDateFormat("yyyyMMdd").format(new Date());
assertFileExists("target/filelanguageinbox/myfile-123-" + date + ".txt");
diff --git a/camel-core/src/test/java/org/apache/camel/component/file/FileProducerJailStartingDirectoryTest.java b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerJailStartingDirectoryTest.java
new file mode 100644
index 0000000..becbf18
--- /dev/null
+++ b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerJailStartingDirectoryTest.java
@@ -0,0 +1,73 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.file;
+
+import org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.junit.Before;
+import org.junit.Test;
+
+public class FileProducerJailStartingDirectoryTest extends ContextTestSupport {
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ deleteDirectory("target/jail");
+ super.setUp();
+ }
+
+ @Test
+ public void testWriteOutsideStartingDirectory() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(0);
+
+ try {
+ template.sendBodyAndHeader("direct:start", "Hello World", Exchange.FILE_NAME, "hello.txt");
+ fail("Should have thrown exception");
+ } catch (Exception e) {
+ IllegalArgumentException iae = assertIsInstanceOf(IllegalArgumentException.class, e.getCause());
+ assertTrue(iae.getMessage().contains("as the filename is jailed to the starting directory"));
+ }
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Test
+ public void testWriteInsideStartingDirectory() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(1);
+
+ template.sendBodyAndHeader("direct:start", "Bye World", Exchange.FILE_NAME, "outbox/bye.txt");
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() throws Exception {
+ return new RouteBuilder() {
+ @Override
+ public void configure() throws Exception {
+ from("direct:start")
+ .setHeader(Exchange.FILE_NAME, simple("../${file:name}"))
+ .to("file:target/jail/outbox")
+ .to("mock:result");
+ }
+ };
+ }
+}
diff --git a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java
index 435a1b4..06b8b38 100644
--- a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java
+++ b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java
@@ -85,7 +85,7 @@ public class FtpProducerExpressionTest extends FtpServerTestSupport {
@Test
public void testProducerComplexByExpression() throws Exception {
// need one extra subdirectory (=foo) to be able to start with .. in the fileName option
- String url = "ftp://admin@localhost:" + getPort() + "/filelanguage/foo?password=admin";
+ String url = "ftp://admin@localhost:" + getPort() + "/filelanguage/foo?password=admin&jailStartingDirectory=false";
String expression = "../filelanguageinbox/myfile-${bean:myguidgenerator.guid}-${date:now:yyyyMMdd}.txt";
template.sendBody(url + "&fileName=" + expression, "Hello World");
diff --git a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerJailStartingDirectoryTest.java b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerJailStartingDirectoryTest.java
new file mode 100644
index 0000000..7cb677a
--- /dev/null
+++ b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerJailStartingDirectoryTest.java
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.file.remote;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.junit.Test;
+
+public class FtpProducerJailStartingDirectoryTest extends FtpServerTestSupport {
+
+ private String getFtpUrl() {
+ return "ftp://admin@localhost:" + getPort() + "/upload/jail?binary=false&password=admin&tempPrefix=.uploading";
+ }
+
+ @Test
+ public void testWriteOutsideStartingDirectory() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(0);
+
+ try {
+ template.sendBodyAndHeader("direct:start", "Hello World", Exchange.FILE_NAME, "hello.txt");
+ fail("Should have thrown exception");
+ } catch (Exception e) {
+ IllegalArgumentException iae = assertIsInstanceOf(IllegalArgumentException.class, e.getCause());
+ assertTrue(iae.getMessage().contains("as the filename is jailed to the starting directory"));
+ }
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Test
+ public void testWriteInsideStartingDirectory() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(1);
+
+ template.sendBodyAndHeader("direct:start", "Bye World", Exchange.FILE_NAME, "jail/bye.txt");
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() throws Exception {
+ return new RouteBuilder() {
+ @Override
+ public void configure() throws Exception {
+ from("direct:start")
+ .setHeader(Exchange.FILE_NAME, simple("../${file:name}"))
+ .to(getFtpUrl())
+ .to("mock:result");
+ }
+ };
+ }
+}
\ No newline at end of file