[backport camel-4.18.x] CAMEL-25704: camel-undertow - refuse a request target or header value with control characters

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);
                 }

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)));

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) {

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}"));
+            }
+        };
+    }
+}

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.