diff --git a/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/main/java/org/apache/tika/pipes/fetcher/http/HttpFetcher.java b/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/main/java/org/apache/tika/pipes/fetcher/http/HttpFetcher.java index 68e2535d2b..efaf6e0106 100644 --- a/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/main/java/org/apache/tika/pipes/fetcher/http/HttpFetcher.java +++ b/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/main/java/org/apache/tika/pipes/fetcher/http/HttpFetcher.java @@ -38,6 +38,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.regex.Pattern; import com.nimbusds.jose.JOSEException; import org.apache.commons.io.IOUtils; @@ -94,6 +95,9 @@ public static HttpFetcher build(ExtensionConfig pluginConfig) throws TikaConfigE } + // RFC 7230 token + private static final Pattern HEADER_NAME = Pattern.compile("[!#$%&'*+.^_`|~0-9A-Za-z-]+"); + private HttpFetcherConfig httpFetcherConfig = new HttpFetcherConfig(); private HttpClientFactory httpClientFactory = new HttpClientFactory(); @@ -175,7 +179,7 @@ private RequestConfig buildRequestConfig() { .build(); } - private void setHttpRequestHeaders(Metadata metadata, HttpGet get) { + private void setHttpRequestHeaders(Metadata metadata, HttpGet get) throws TikaException { if (!StringUtils.isBlank(userAgent)) { get.setHeader(USER_AGENT, userAgent); } @@ -194,16 +198,34 @@ private void setHttpRequestHeaders(Metadata metadata, HttpGet get) { } } - private static void parseHeaderAndPutOnRequest(HttpGet get, String httpRequestHeader) { + private static void parseHeaderAndPutOnRequest(HttpGet get, String httpRequestHeader) + throws TikaException { String[] parts = httpRequestHeader .trim().split(":", 2); if (parts.length >= 2) { String key = parts[0].trim(); String value = parts[1].trim(); + checkHeader(key, value); get.setHeader(key, value); } } + /** + * httpcore folds CR and LF in a value into spaces but writes the name as given, so a + * name from per-request config that carries CRLF would inject request lines. + */ + private static void checkHeader(String name, String value) throws TikaException { + if (!HEADER_NAME.matcher(name).matches()) { + throw new TikaException("Invalid HTTP header name: " + name); + } + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if ((c < 0x20 && c != '\t') || c == 0x7f) { + throw new TikaException("Invalid control character in HTTP header " + name); + } + } + } + private HttpFetcherRequestConfig getAdditionalHttpFetcherConfig(ParseContext parseContext) throws TikaConfigException { if (parseContext.hasJsonConfig("http-fetcher")) { @@ -234,14 +256,18 @@ private void putAdditionalHeadersOnRequest(HttpFetcherRequestConfig additionalFe httpGet.setHeader(USER_AGENT, httpFetcherConfig.getUserAgent()); } if (additionalFetcherConfig != null && additionalFetcherConfig.getHttpRequestHeaders() != null) { - additionalFetcherConfig.getHttpRequestHeaders() - .getHeaders() - .forEach(httpGet::addHeader); + for (Map.Entry e : additionalFetcherConfig.getHttpRequestHeaders() + .getHeaders().entries()) { + checkHeader(e.getKey(), e.getValue()); + httpGet.addHeader(e.getKey(), e.getValue()); + } } if (httpFetcherConfig.getHttpRequestHeaders() != null) { - httpFetcherConfig.getHttpRequestHeaders() - .getHeaders() - .forEach(httpGet::addHeader); + for (Map.Entry e : httpFetcherConfig.getHttpRequestHeaders() + .getHeaders().entries()) { + checkHeader(e.getKey(), e.getValue()); + httpGet.addHeader(e.getKey(), e.getValue()); + } } if (jwtGenerator != null) { try { @@ -252,7 +278,8 @@ private void putAdditionalHeadersOnRequest(HttpFetcherRequestConfig additionalFe } } - private static void placeHeaderOnGetRequest(HttpGet get, String httpRequestHeader) { + private static void placeHeaderOnGetRequest(HttpGet get, String httpRequestHeader) + throws TikaException { int idxOfEquals = httpRequestHeader.indexOf(':'); if (idxOfEquals == -1) { return; @@ -263,6 +290,7 @@ private static void placeHeaderOnGetRequest(HttpGet get, String httpRequestHeade String headerValue = httpRequestHeader .substring(idxOfEquals + 1) .trim(); + checkHeader(headerKey, headerValue); get.setHeader(headerKey, headerValue); } diff --git a/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/test/java/org/apache/tika/pipes/fetcher/http/HttpFetcherTest.java b/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/test/java/org/apache/tika/pipes/fetcher/http/HttpFetcherTest.java index fde8f185eb..932fc858fd 100644 --- a/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/test/java/org/apache/tika/pipes/fetcher/http/HttpFetcherTest.java +++ b/tika-pipes/tika-pipes-plugins/tika-pipes-http/src/test/java/org/apache/tika/pipes/fetcher/http/HttpFetcherTest.java @@ -35,6 +35,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import java.util.zip.GZIPInputStream; @@ -261,6 +262,41 @@ public String getReasonPhrase() { } + /** + * httpcore writes a header name as given (only a value has CR and LF folded), so a + * per-request header name carrying CRLF would inject request lines. + */ + @Test + public void testHeadersWithCrLfAreRefused() throws Exception { + HttpResponse response = buildMockResponse(HttpStatus.SC_OK, IOUtils.toInputStream(CONTENT, Charset.defaultCharset())); + HttpClient httpClient = Mockito.mock(HttpClient.class); + Mockito.when(httpClient.execute(ArgumentMatchers.any(HttpUriRequest.class), ArgumentMatchers.any(HttpContext.class))) + .thenReturn(response); + httpFetcher.setHttpClient(httpClient); + + ParseContext nameContext = new ParseContext(); + nameContext.setJsonConfig("http-fetcher", OBJECT_MAPPER.writeValueAsString( + Map.of("httpRequestHeaders", Map.of("map", Map.of( + "X-Test\r\nInjected: yes\r\n\r\nPOST /evil HTTP/1.1\r\nHost: h", + List.of("v")))))); + assertThrows(TikaException.class, + () -> httpFetcher.fetch("http://localhost", new Metadata(), nameContext)); + + ParseContext valueContext = new ParseContext(); + valueContext.setJsonConfig("http-fetcher", OBJECT_MAPPER.writeValueAsString( + Map.of("httpRequestHeaders", Map.of("map", Map.of( + "X-Test", List.of("v\r\nInjected: yes")))))); + assertThrows(TikaException.class, + () -> httpFetcher.fetch("http://localhost", new Metadata(), valueContext)); + + Metadata metadata = new Metadata(); + metadata.set(Property.externalText("httpRequestHeaders"), "X-Test\r\nInjected: yes: v"); + assertThrows(TikaException.class, + () -> httpFetcher.fetch("http://localhost", metadata, new ParseContext())); + + Mockito.verifyNoInteractions(httpClient); + } + @Test @Disabled("requires network connectivity") public void testRedirect() throws Exception {