diff --git a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/DefaultUndertowHttpBinding.java b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/DefaultUndertowHttpBinding.java index a669de857ba78..5890436be2ed0 100644 --- a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/DefaultUndertowHttpBinding.java +++ b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/DefaultUndertowHttpBinding.java @@ -407,7 +407,8 @@ public Object toHttpRequest(ClientRequest clientRequest, Message message) { // set the content type in the response. String contentType = MessageHelper.getContentType(message); if (contentType != null) { - // set content-type + // set content-type (written verbatim by the client, so validate it like any other header value) + UndertowHelper.checkWriteable("Content-Type header", contentType); requestHeaders.put(Headers.CONTENT_TYPE, contentType); LOG.trace("Content-Type: {}", contentType); } @@ -424,6 +425,10 @@ public Object toHttpRequest(ClientRequest clientRequest, Message message) { String headerValue = tc.convertTo(String.class, it.next()); if (headerValue != null && headerFilterStrategy != null && !headerFilterStrategy.applyFilterToCamelHeaders(key, headerValue, message.getExchange())) { + // the HTTP client writes each character of the value as a single byte with no validation, so a + // CR or LF (or a character that narrows to one) would break the value out of its header line; + // reject such a value rather than send a malformed request + UndertowHelper.checkWriteable("value of HTTP header " + key, headerValue); LOG.trace("HTTP-Header: {}={}", key, headerValue); requestHeaders.add(new HttpString(key), headerValue); } diff --git a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowHelper.java b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowHelper.java index 460d12a30664d..4a97691cfe873 100644 --- a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowHelper.java +++ b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowHelper.java @@ -150,6 +150,31 @@ public static HttpString createMethod(Exchange exchange, UndertowEndpoint endpoi return answer; } + /** + * Rejects a value that cannot be written to the HTTP request line or a header line as it is. The Undertow client + * writes each character of the request path, query and header values as a single byte without any validation, so a + * CR or LF would break the request into further lines or headers, and a character above {@code 0xFF} is narrowed to + * a single, different byte (for example {@code U+010A} would be written as {@code LF}). A control character, a + * {@code DEL} or a character above {@code 0xFF} is therefore refused; {@code SP} and {@code HTAB}, which do not + * change the framing, are allowed. + * + * @param what a short description of the value, used in the error message + * @param value the value to check, may be {@code null} + * @throws IllegalArgumentException if the value holds a character that cannot be written as it is + */ + public static void checkWriteable(String what, String value) { + if (value == null) { + return; + } + for (int i = 0; i < value.length(); i++) { + char ch = value.charAt(i); + if (ch > 0xFF || ch == 0x7F || (ch < 0x20 && ch != '\t')) { + throw new IllegalArgumentException( + "Illegal character 0x" + Integer.toHexString(ch) + " in " + what); + } + } + } + public static URI makeHttpURI(String httpURI) { return makeHttpURI( URI.create(UnsafeUriCharactersEncoder.encodeHttpURI(httpURI))); diff --git a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowProducer.java b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowProducer.java index 06a90683f806b..9956b6dc4ad8a 100644 --- a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowProducer.java +++ b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowProducer.java @@ -96,18 +96,21 @@ public boolean process(final Exchange camelExchange, final AsyncCallback callbac /* not a WebSocket */ final URI uri; final HttpString method; + final String pathAndQuery; try { final String exchangeUri = UndertowHelper.createURL(camelExchange, getEndpoint()); uri = UndertowHelper.createURI(camelExchange, exchangeUri, getEndpoint()); method = UndertowHelper.createMethod(camelExchange, endpoint, camelExchange.getIn().getBody() != null); - } catch (final URISyntaxException e) { + pathAndQuery = URISupport.pathAndQueryOf(uri); + // the client writes the request target byte by byte with no validation, so a CR or LF (for example from a + // percent-encoded control character in a header) would break the request line; refuse it instead + UndertowHelper.checkWriteable("HTTP request target", pathAndQuery); + } catch (final URISyntaxException | IllegalArgumentException e) { camelExchange.setException(e); callback.done(true); return true; } - final String pathAndQuery = URISupport.pathAndQueryOf(uri); - final UndertowHttpBinding undertowHttpBinding = endpoint.getUndertowHttpBinding(); final CookieHandler cookieHandler = endpoint.getCookieHandler(); @@ -130,19 +133,30 @@ public boolean process(final Exchange camelExchange, final AsyncCallback callbac final HeaderMap requestHeaders = request.getRequestHeaders(); - // Set the Host header - final Message message = camelExchange.getIn(); - final String host = message.getHeader(UndertowConstants.HOST_STRING, String.class); - if (endpoint.isPreserveHostHeader()) { - requestHeaders.put(Headers.HOST, Optional.ofNullable(host).orElseGet(uri::getAuthority)); - } else { - requestHeaders.put(Headers.HOST, uri.getAuthority()); - } - cookieHeaders.forEach((key, values) -> { - requestHeaders.putAll(HttpString.tryFromString(key), values); - }); + final Object body; + try { + // Set the Host header. When preserveHostHeader is set this is the caller's Host header, which the client + // writes verbatim, so it is validated like any other header value. + final Message message = camelExchange.getIn(); + final String host = message.getHeader(UndertowConstants.HOST_STRING, String.class); + if (endpoint.isPreserveHostHeader()) { + final String hostHeader = Optional.ofNullable(host).orElseGet(uri::getAuthority); + UndertowHelper.checkWriteable("Host header", hostHeader); + requestHeaders.put(Headers.HOST, hostHeader); + } else { + requestHeaders.put(Headers.HOST, uri.getAuthority()); + } + cookieHeaders.forEach((key, values) -> { + requestHeaders.putAll(HttpString.tryFromString(key), values); + }); - final Object body = undertowHttpBinding.toHttpRequest(request, camelExchange.getIn()); + body = undertowHttpBinding.toHttpRequest(request, camelExchange.getIn()); + } catch (final IllegalArgumentException e) { + // a Host or other header value that cannot be written to a header line (see UndertowHelper.checkWriteable) + camelExchange.setException(e); + callback.done(true); + return true; + } final UndertowClientCallback clientCallback; final boolean streaming = getEndpoint().isUseStreaming(); if (streaming && body instanceof InputStream) { diff --git a/components/camel-undertow/src/test/java/org/apache/camel/component/undertow/UndertowProducerEncodedUriTest.java b/components/camel-undertow/src/test/java/org/apache/camel/component/undertow/UndertowProducerEncodedUriTest.java new file mode 100644 index 0000000000000..23b947215fa55 --- /dev/null +++ b/components/camel-undertow/src/test/java/org/apache/camel/component/undertow/UndertowProducerEncodedUriTest.java @@ -0,0 +1,113 @@ +/* + * 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.undertow; + +import org.apache.camel.Exchange; +import org.apache.camel.builder.RouteBuilder; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The undertow producer must not write a request path, query or header value that it cannot put on the wire as it is: a + * percent-encoded CR/LF in {@code CamelHttpPath} or {@code CamelHttpQuery}, or a CR/LF in a header value, is refused + * rather than written into the request line or a header line. An ordinary query is still sent. + */ +class UndertowProducerEncodedUriTest extends BaseUndertowTest { + + @Test + void rejectsEncodedCrlfInQuery() { + Exchange out = template.request("undertow:http://localhost:{{port}}/echo", + e -> e.getIn().setHeader(Exchange.HTTP_QUERY, "q=x%0D%0AX-Injected:%201")); + assertRejected(out, "request target"); + } + + @Test + void rejectsEncodedCrlfInPath() { + Exchange out = template.request("undertow:http://localhost:{{port}}/echo", + e -> e.getIn().setHeader(Exchange.HTTP_PATH, "a%0D%0Ab")); + assertRejected(out, "request target"); + } + + @Test + void rejectsNarrowingCharacterInQuery() { + // U+010A is above 0xFF; the client would narrow it to the single byte 0x0A (LF). It is refused before that. + Exchange out = template.request("undertow:http://localhost:{{port}}/echo", + e -> e.getIn().setHeader(Exchange.HTTP_QUERY, "q=Ċ")); + assertRejected(out, "request target"); + } + + @Test + void rejectsHeaderValueWithCrlf() { + Exchange out = template.request("undertow:http://localhost:{{port}}/echo", + e -> e.getIn().setHeader("MyHeader", "value\r\nX-Injected: 1")); + assertRejected(out, "HTTP header"); + } + + @Test + void rejectsCrlfInPreservedHostHeader() { + // with preserveHostHeader the caller's Host header is written verbatim, so it is validated too + Exchange out = template.request("undertow:http://localhost:{{port}}/echo?preserveHostHeader=true", + e -> e.getIn().setHeader("Host", "localhost\r\nX-Injected: 1")); + assertRejected(out, "Host header"); + } + + @Test + void sendsAnOrdinaryQuery() { + String out = template.requestBodyAndHeader("undertow:http://localhost:{{port}}/echo", + null, Exchange.HTTP_QUERY, "q=hello", String.class); + assertEquals("q=hello", out); + } + + @Test + void sendsAHeaderWithTabAndLatin1() { + // a tab, a space and a Latin-1 (obs-text) character do not change the request framing, so they are NOT refused + Exchange out = template.request("undertow:http://localhost:{{port}}/echo", + e -> e.getIn().setHeader("MyHeader", "café\tau lait")); + assertFalse(out.isFailed(), "a header value with a tab and a Latin-1 character must not be refused"); + } + + private static void assertRejected(Exchange out, String expectedInMessage) { + assertTrue(out.isFailed(), "the exchange should have failed"); + Throwable cause = out.getException(); + assertNotNull(cause); + boolean found = false; + while (cause != null) { + if (cause instanceof IllegalArgumentException && cause.getMessage() != null + && cause.getMessage().contains(expectedInMessage)) { + found = true; + break; + } + cause = cause.getCause(); + } + assertTrue(found, "expected an IllegalArgumentException mentioning '" + expectedInMessage + "'"); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + from("undertow:http://localhost:{{port}}/echo") + .setBody(simple("${header.CamelHttpQuery}")); + } + }; + } +} diff --git a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc index 0f189294ba46f..94b2681660c9d 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc @@ -1812,3 +1812,14 @@ link. Links resolving inside the starting directory work as before. Set `jailSta links to other directories (this also turns off the check of `../` in producer file names). A skipped file is logged at WARN the first time and at DEBUG on later polls; this log-level change also applies to the remote file consumers, which keep their existing lexical check. + +=== camel-undertow - the producer refuses a request target or header value with control characters + +The undertow producer writes the path, query and header values of the outgoing request to the socket one character at +a time, without validating them. A carriage return or line feed therefore broke the request into further lines, and a +character above `0xFF` was written as a single, different byte (for example `U+010A` became a line feed). The producer +now refuses to send a request path, query or header value that contains a control character or a character above +`0xFF`, and fails the exchange with an `IllegalArgumentException` instead. A space and a tab, which do not change the +framing, are still allowed. Routes that put a percent-encoded control character (such as `%0D%0A`) into the +`CamelHttpPath` or `CamelHttpQuery` header, which the producer decodes before building the request, are the ones most +likely to be affected.