Skip to content

Commit

Permalink
Issue #2212 Enhances validation of HTTP header names
Browse files Browse the repository at this point in the history 
+ Added the validation of HTTP header values
+ Name and value's validation are provided as options. (Improved based on carryel#1 @breakponchito)
  • Loading branch information
@carryel
carryel committed Nov 18, 2024
1 parent be0e4a1 commit 6bb7eab
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 36 deletions.
75 changes: 39 additions & 36 deletions modules/http/src/main/java/org/glassfish/grizzly/http/HttpCodecFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
}
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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");
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 9 additions & 0 deletions modules/http/src/main/java/org/glassfish/grizzly/http/util/CookieHeaderParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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 {
Expand Down

0 comments on commit 6bb7eab

Please sign in to comment.