From b3935a1cbf1cd50473b9eb67e334ab94a34d82f1 Mon Sep 17 00:00:00 2001 From: Bongjae Chang Date: Mon, 18 Nov 2024 14:09:20 +0900 Subject: [PATCH] Issue #2212 Enhances validation of HTTP header names + Added the validation of HTTP header values + Name and value's validation are provided as options. (Improved based on https://github.com/carryel/grizzly/pull/1 @breakponchito) --- .../grizzly/http/HttpCodecFilter.java | 75 ++++++++++--------- .../grizzly/http/util/CookieHeaderParser.java | 9 +++ .../grizzly/http/HttpRequestParseTest.java | 19 +++++ 3 files changed, 67 insertions(+), 36 deletions(-) diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java index b29c1309c..bc01380bd 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java @@ -22,14 +22,11 @@ import static org.glassfish.grizzly.utils.Charsets.ASCII_CHARSET; import java.io.IOException; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Pattern; import org.glassfish.grizzly.Buffer; import org.glassfish.grizzly.Connection; import org.glassfish.grizzly.Grizzly; @@ -120,8 +117,6 @@ public abstract class HttpCodecFilter extends HttpBaseFilter implements Monitori private static final boolean isStrictHeaderNameValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_NAME_VALIDATION_RFC_9110)); private static final boolean isStrictHeaderValueValidationSet = Boolean.parseBoolean(System.getProperty(STRICT_HEADER_VALUE_VALIDATION_RFC_9110)); - - private static final String REGEX_RFC_9110_INVALID_CHARACTERS = "(\\\\n)|(\\\\0)|(\\\\r)|(\\\\x00)|(\\\\x0A)|(\\\\x0D)"; /** * File cache probes @@ -830,6 +825,20 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP while (offset < limit) { final byte b = input[offset]; if (b == Constants.CR) { + if (isStrictHeaderValueValidationSet) { + if (offset + 1 < limit) { + final byte b2 = input[offset + 1]; + if (b2 == Constants.LF) { + // Continue for next parsing without the validation + offset++; + continue; + } + } else { + // not enough data + parsingState.offset = offset - arrayOffs; + return -1; + } + } } else if (b == Constants.LF) { // Check if it's not multi line header if (offset + 1 < limit) { @@ -842,10 +851,6 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP parsingState.offset = offset + 1 - arrayOffs; finalizeKnownHeaderValues(httpHeader, parsingState, input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2); parsingState.headerValueStorage.setBytes(input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2); - if (isStrictHeaderValueValidationSet) { - //make validation with regex mode - validateRFC9110Characters(input, arrayOffs + parsingState.start, arrayOffs + parsingState.checkpoint2); - } return 0; } } @@ -866,35 +871,15 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP } parsingState.checkpoint2 = parsingState.checkpoint; } - - offset++; - } - parsingState.offset = offset - arrayOffs; - return -1; - } - private static void validateRFC9110Characters(final byte[] headerValueContent, int start, int end) { - if (headerValueContent != null) { - if (isInvalidCharacterAvailable(start, end, headerValueContent)) { + if (isStrictHeaderValueValidationSet && !CookieHeaderParser.isText(b)) { throw new IllegalStateException( - "An invalid character NUL, LF or CR found in the header value: " + headerValueContent.toString()); + "An invalid character 0x" + Integer.toHexString(b) + " was found in the header value"); } + offset++; } - } - - /** - * This method evaluates the String from the bytes that contains Header Value and validates if contains literal value - * of \n, \r or \0 , in case any of those characters are available return true - * @param start index of the starting point to extract characters from the byte array - * @param end index of the end point to extract characters from the byte array - * @param bytesFromByteChunk represents the bytes from the request message to be processed - * @return Boolean true if any of those characters are available - */ - private static boolean isInvalidCharacterAvailable(int start, int end, byte[] bytesFromByteChunk) { - byte[] bytesFromHeaderValue = Arrays.copyOfRange(bytesFromByteChunk, start, end); - String uft8String = new String(bytesFromHeaderValue, StandardCharsets.UTF_8); - Pattern pattern = Pattern.compile(REGEX_RFC_9110_INVALID_CHARACTERS); - return pattern.matcher(uft8String).find(); + parsingState.offset = offset - arrayOffs; + return -1; } private static void finalizeKnownHeaderNames(final HttpHeader httpHeader, final HeaderParsingState parsingState, final byte[] input, final int start, @@ -1095,7 +1080,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim b -= Constants.LC_OFFSET; } input.put(offset, b); - } else if (b == Constants.CR) { + } else if (isStrictHeaderNameValidationSet && b == Constants.CR) { parsingState.offset = offset; final int eol = checkEOL(parsingState, input); if (eol == 0) { // EOL @@ -1107,7 +1092,7 @@ protected int parseHeaderName(final HttpHeader httpHeader, final MimeHeaders mim } } - if (!CookieHeaderParser.isToken(b)) { + if (isStrictHeaderNameValidationSet && !CookieHeaderParser.isToken(b)) { throw new IllegalStateException( "An invalid character 0x" + Integer.toHexString(b) + " was found in the header name"); } @@ -1129,6 +1114,20 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP while (offset < limit) { final byte b = input.get(offset); if (b == Constants.CR) { + if (isStrictHeaderValueValidationSet) { + if (offset + 1 < limit) { + final byte b2 = input.get(offset + 1); + if (b2 == Constants.LF) { + // Continue for next parsing without the validation + offset++; + continue; + } + } else { + // not enough data + parsingState.offset = offset; + return -1; + } + } } else if (b == Constants.LF) { // Check if it's not multi line header if (offset + 1 < limit) { @@ -1162,6 +1161,10 @@ protected static int parseHeaderValue(final HttpHeader httpHeader, final HeaderP parsingState.checkpoint2 = parsingState.checkpoint; } + if (isStrictHeaderValueValidationSet && !CookieHeaderParser.isText(b)) { + throw new IllegalStateException( + "An invalid character 0x" + Integer.toHexString(b) + " was found in the header value"); + } offset++; } parsingState.offset = offset; diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java index b6ea633b1..1275a06af 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java @@ -248,6 +248,15 @@ public static boolean isToken(int c) { } } + public static boolean isText(int c) { + // Fast for correct values, slower for incorrect ones + try { + return isText[c]; + } catch (ArrayIndexOutOfBoundsException ex) { + return false; + } + } + /** * Custom implementation that skips many of the safety checks in diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java index af909c529..a0fe1ee60 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java @@ -124,6 +124,15 @@ public void testDisallowedHeaders() { } public void testDisallowedCharactersForHeaderContentValues() { + final StringBuilder sb = new StringBuilder("GET / HTTP/1.1\r\n"); + sb.append("Host: localhost\r\n"); + sb.append("Some-Header: some-"); + // valid header values + sb.append(new char[]{'\t', ' ', '\"', '(', ')', '/', ';', '<', '=', '>', '?', '@', '[', 0x5c, ']', '{', '}'}) + .append("\r\n"); + sb.append("\r\n"); + doTestDecoder(sb.toString(), 128); + try { doTestDecoder("GET /index.html HTTP/1.1\nHost: loca\\rlhost\nContent -Length: 1234\n\n", 128); fail("Bad HTTP headers exception had to be thrown"); @@ -142,6 +151,16 @@ public void testDisallowedCharactersForHeaderContentValues() { } catch (IllegalStateException e) { // expected } + + final char[] invalidChars = new char[]{0x00, 0x01, 0x02, '\r'}; + for (final char ch : invalidChars) { + try { + doTestDecoder("GET /index.html HTTP/1.1\nHost: localhost\nSome-Header: some-" + ch + "value\n\n", 128); + fail("Bad HTTP headers exception had to be thrown"); + } catch (IllegalStateException e) { + // expected + } + } } public void testIgnoredHeaders() throws Exception {