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 <[email protected]> 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
