Skip to content

Commit c74ea86

Browse files
authored
Security fixes (#2962)
* Fix possible desync caused by case-sensitive comparison for Transfer-Encoding header * Add validation to prevent HTTP header injection attacks Implement control character validation (CR, LF, NUL) in `HeaderField` setters to prevent HTTP header injection. Update constructors to use validated setters. Add comprehensive tests covering header value and name injection attempts. * Add HEADER serialization to prevent HTTP header injection Introduce `HEADER` serialization type that strips CR, LF, and NUL characters to prevent header injection attacks. Apply header encoding by default in `SetHeaderInterceptor` through `TemplateExchangeExpression`. Remove redundant error handling for IllegalArgumentException in `AbstractSetterInterceptor`. * Rename HEADER serialization to HEADERVALUE and escape instead of strip control characters Change `HEADER` to `HEADERVALUE` to better reflect its purpose for header values specifically. Update encoding behavior from stripping CR, LF, and NUL characters to escaping them as `\r`, `\n`, and `\0` respectively. Fix `isChunked()` to check all Transfer-Encoding values instead of only the first one. * Refactor HeaderField to use centralized header value serialization Move control character validation from `HeaderField` to `HEADERVALUE_SERIALIZATION` function for centralized header value encoding. Remove redundant `containsControlChar()` method and inline validation logic. Simplify `setValue()` to delegate sanitization to the serialization function. * Reorder Serialization enum documentation to match declaration order Move enum documentation entries to align with the declaration order in the code. Place JSON, XML, URL, TEXT, SEGMENT, and HEADERVALUE descriptions in sequence matching their enum definition. * Change header injection tests from expecting exceptions to verifying escaped control characters Update sanitization tests to verify that control characters (CR, LF, NUL) are escaped rather than rejected. Replace `assertThrows` assertions with `assertEquals` checks confirming values are sanitized with backslash escaping.
1 parent 537418e commit c74ea86

7 files changed

Lines changed: 95 additions & 20 deletions

File tree

core/src/main/java/com/predic8/membrane/core/http/Header.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public void setProxyAuthorization(String value) {
318318
}
319319

320320
public boolean isChunked() {
321-
return CHUNKED.equals(getFirstValue(TRANSFER_ENCODING));
321+
return getSingleValues(TRANSFER_ENCODING).anyMatch(CHUNKED::equalsIgnoreCase);
322322
}
323323

324324
public long getContentLength() {

core/src/main/java/com/predic8/membrane/core/http/HeaderField.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,22 @@
1515

1616
package com.predic8.membrane.core.http;
1717

18-
import static com.predic8.membrane.annot.Constants.*;
18+
import static com.predic8.membrane.annot.Constants.CRLF;
19+
import static com.predic8.membrane.core.util.text.SerializationFunction.HEADERVALUE_SERIALIZATION;
1920

2021
public class HeaderField {
2122

2223
private HeaderName headerName;
2324
private String value;
2425

2526
public HeaderField(HeaderName headerName,String value) {
26-
this.headerName = headerName;
27-
this.value = value;
27+
setHeaderName(headerName);
28+
setValue(value);
2829
}
2930

3031
public HeaderField(String line) {
31-
headerName = new HeaderName(getName(line));
32-
value = getValue(line);
32+
setHeaderName(new HeaderName(getName(line)));
33+
setValue(getValue(line));
3334
}
3435

3536
private String getValue(String line) {
@@ -52,12 +53,14 @@ public HeaderField(HeaderField element) {
5253
public String getValue() {
5354
return value;
5455
}
56+
5557
public void setValue(String value) {
56-
this.value = value;
58+
this.value = HEADERVALUE_SERIALIZATION.apply(value);
5759
}
5860
public HeaderName getHeaderName() {
5961
return headerName;
6062
}
63+
6164
public void setHeaderName(HeaderName headerName) {
6265
this.headerName = headerName;
6366
}
@@ -70,4 +73,4 @@ public String toString(){
7073
public int estimateHeapSize() {
7174
return 2*(4 + headerName.toString().length() + (value == null ? 0 : value.length()));
7275
}
73-
}
76+
}

core/src/main/java/com/predic8/membrane/core/interceptor/lang/SetHeaderInterceptor.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313
limitations under the License. */
1414
package com.predic8.membrane.core.interceptor.lang;
1515

16-
import com.predic8.membrane.annot.*;
17-
import com.predic8.membrane.core.exchange.*;
18-
import com.predic8.membrane.core.lang.*;
19-
import com.predic8.membrane.core.util.text.*;
16+
import com.predic8.membrane.annot.MCElement;
17+
import com.predic8.membrane.core.exchange.Exchange;
18+
import com.predic8.membrane.core.lang.ExchangeExpression;
19+
import com.predic8.membrane.core.lang.TemplateExchangeExpression;
20+
21+
import static com.predic8.membrane.core.util.text.SerializationFunction.HEADERVALUE_SERIALIZATION;
2022

2123
/**
2224
* @description Set HTTP header on the current message.
@@ -41,6 +43,11 @@ protected boolean shouldSetValue(Exchange exc, Flow flow) {
4143
return true;
4244
}
4345

46+
@Override
47+
protected ExchangeExpression getExchangeExpression() {
48+
return TemplateExchangeExpression.newInstance(this, language, expression, router, HEADERVALUE_SERIALIZATION);
49+
}
50+
4451
@Override
4552
protected void setValue(Exchange exc, Flow flow, Object value) {
4653
exc.getMessage(flow).getHeader().setValue(fieldName, value.toString());

core/src/main/java/com/predic8/membrane/core/util/text/SerializationFunction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,5 @@ public interface SerializationFunction extends Function<Object, String> {
3030
SerializationFunction TEXT_SERIALIZATION = ToTextSerializer::toText;
3131
SerializationFunction URL_SERIALIZATION = ToURLSerializer::toURL;
3232
SerializationFunction SEGMENT_SERIALIZATION = SerializationUtil::pathEncode;
33-
33+
SerializationFunction HEADERVALUE_SERIALIZATION = SerializationUtil::headerValueEncode;
3434
}

core/src/main/java/com/predic8/membrane/core/util/text/SerializationUtil.java

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,20 +35,22 @@ private SerializationUtil() {
3535
* <p/>
3636
* The strategies include:
3737
* <p/>
38-
* - {@code URL}: Encodes strings for safe inclusion in a URL, replacing spaces and
39-
* other special characters with their percent-encoded counterparts (e.g., SPACE -> +).
4038
* - {@code JSON}: Serializes s for safe inclusion in a JSON context.
4139
* - {@code XML}: Serializes for safe inclusion in an XML context using XML 1.1 rules.
42-
* - {@code SEGMENT}: Encodes as safe URI path segments, ensuring they do not introduce
40+
* - {@code URL}: Encodes strings for safe inclusion in a URL, replacing spaces and
41+
* other special characters with their percent-encoded counterparts (e.g., SPACE -> +).
4342
* - {@code TEXT}: Serializes as plain text, without any encoding.
43+
* - {@code SEGMENT}: Encodes as safe URI path segments, ensuring they do not introduce
4444
* path separators, query delimiters, or other unsafe characters, as per RFC 3986.
45+
* - {@code HEADERVALUE}: Escapes CR, LF, and NUL to secure header values.
4546
*/
4647
public enum Serialization {
48+
JSON,
49+
XML,
4750
TEXT,
4851
URL,
4952
SEGMENT,
50-
JSON,
51-
XML
53+
HEADERVALUE
5254
}
5355

5456
public static Optional<SerializationFunction> getSerialization(String mimeType) {
@@ -67,11 +69,12 @@ public static Optional<SerializationFunction> getSerialization(String mimeType)
6769

6870
public static SerializationFunction getSerialization(Serialization serialization) {
6971
return switch (serialization) {
72+
case JSON -> JSON_SERIALIZATION;
73+
case XML -> XML_SERIALIZATION;
7074
case TEXT -> TEXT_SERIALIZATION;
7175
case URL -> URL_SERIALIZATION;
7276
case SEGMENT -> SEGMENT_SERIALIZATION;
73-
case JSON -> JSON_SERIALIZATION;
74-
case XML -> XML_SERIALIZATION;
77+
case HEADERVALUE -> HEADERVALUE_SERIALIZATION;
7578
};
7679
}
7780

@@ -135,4 +138,31 @@ public static String pathEncode(Object value) {
135138

136139
return out.toString();
137140
}
141+
142+
/**
143+
* Encodes the given value so it can be safely used as a header value
144+
* by escaping characters that would enable HTTP header injection:
145+
* CR becomes {@code \r}, LF becomes {@code \n}, and NUL becomes {@code \0}.
146+
*
147+
* @param value the value to encode; may be {@code null}
148+
* @return a string with CR, LF, and NUL characters escaped; empty string if {@code value} is null
149+
*/
150+
public static String headerValueEncode(Object value) {
151+
if (value == null) return "";
152+
153+
String s = value.toString();
154+
var out = new StringBuilder(s.length());
155+
156+
for (int i = 0; i < s.length(); i++) {
157+
char c = s.charAt(i);
158+
switch (c) {
159+
case '\r' -> out.append("\\r");
160+
case '\n' -> out.append("\\n");
161+
case '\0' -> out.append("\\0");
162+
default -> out.append(c);
163+
}
164+
}
165+
166+
return out.toString();
167+
}
138168
}

core/src/test/java/com/predic8/membrane/core/http/HeaderTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,4 +262,29 @@ void unique() {
262262
assertEquals("1, 2", h.getValuesAsString("X-Foo"));
263263
assertEquals("3, 4", h.getValuesAsString("X-BAR"));
264264
}
265+
266+
@Test
267+
void sanitization() {
268+
Header h = new Header();
269+
270+
h.add("X-CRLF", "value\r\nInjected: evil");
271+
h.add("X-CR", "value\rInjected: evil");
272+
h.add("X-LF", "value\nInjected: evil");
273+
h.add("X-NUL", "value\u0000Injected");
274+
assertEquals("value\\r\\nInjected: evil", h.getFirstValue("X-CRLF"));
275+
assertEquals("value\\rInjected: evil", h.getFirstValue("X-CR"));
276+
assertEquals("value\\nInjected: evil", h.getFirstValue("X-LF"));
277+
assertEquals("value\\0Injected", h.getFirstValue("X-NUL"));
278+
279+
h.add("X-Bar", "ok");
280+
h.setValue("X-Bar", "evil\r\nInjected: x");
281+
assertEquals("evil\\r\\nInjected: x", h.getFirstValue("X-Bar"));
282+
283+
HeaderField field = h.getAllHeaderFields()[0];
284+
field.setValue("evil\r\nInjected: x");
285+
assertEquals("evil\\r\\nInjected: x", field.getValue());
286+
287+
h.add("X-Normal", "regular value");
288+
assertEquals("regular value", h.getFirstValue("X-Normal"));
289+
}
265290
}

core/src/test/java/com/predic8/membrane/core/interceptor/lang/SetHeaderInterceptorSpELTest.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,16 @@ void notIfAbsentCaseDiff() {
147147
assertEquals("42", getHeader("x-FoO"));
148148
}
149149

150+
@Test
151+
@DisplayName("CR, LF, and NUL in interpolated values are escaped to prevent header injection")
152+
void escapesControlCharsInInterpolation() {
153+
exchange.setProperty("dirty", "evil\r\nInjected: yes\0end");
154+
interceptor.setValue("${properties['dirty']}");
155+
interceptor.init(router);
156+
assertEquals(CONTINUE, interceptor.handleRequest(exchange));
157+
assertEquals("evil\\r\\nInjected: yes\\0end", getHeader("x-bar"));
158+
}
159+
150160
@Test
151161
void failOnErrorTrue() {
152162
interceptor.setValue("42${wrong}");

0 commit comments

Comments
 (0)