From c9b3122b6ca1e7f487c0f79df24f4bac0b9f6817 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Mon, 9 Mar 2009 21:05:49 +0000 Subject: [PATCH] More strict validation on HTTP headers to defend against HTTP response splitting atack --- .../handler/codec/http/DefaultCookie.java | 2 +- .../codec/http/DefaultHttpMessage.java | 73 ++++++++++++++++++- .../netty/handler/codec/http/HttpMessage.java | 2 +- 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java b/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java index f703f6aed5..cb141765ad 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java @@ -61,7 +61,7 @@ public class DefaultCookie implements Cookie { case '\t': case '\r': case '\n': case '\f': case 0x0b: // Vertical tab throw new IllegalArgumentException( - "name contains one of the following characters: " + + "name contains one of the following prohibited characters: " + "=,; \\t\\r\\n\\v\\f: " + name); } } diff --git a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java index 47fd1804a7..bfc41ed1ca 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java @@ -22,6 +22,7 @@ package org.jboss.netty.handler.codec.http; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -50,6 +51,8 @@ public class DefaultHttpMessage implements HttpMessage { } public void addHeader(final String name, final String value) { + validateHeaderName(name); + validateHeaderValue(value); if (value == null) { throw new NullPointerException("value is null"); } @@ -60,6 +63,8 @@ public class DefaultHttpMessage implements HttpMessage { } public void setHeader(final String name, final String value) { + validateHeaderName(name); + validateHeaderValue(value); if (value == null) { throw new NullPointerException("value"); } @@ -69,11 +74,71 @@ public class DefaultHttpMessage implements HttpMessage { headers.put(name, values); } - public void setHeader(final String name, final List values) { - if (values == null || values.size() == 0) { - throw new NullPointerException("no values present"); + public void setHeader(final String name, final Iterable values) { + validateHeaderName(name); + if (values == null) { + throw new NullPointerException("values"); + } + + int nValues = 0; + for (String v: values) { + validateHeaderValue(v); + nValues ++; + } + + if (nValues == 0) { + throw new IllegalArgumentException("values is empty."); + } + + if (values instanceof List) { + headers.put(name, (List) values); + } else { + List valueList = new LinkedList(); + for (String v: values) { + valueList.add(v); + } + headers.put(name, valueList); + } + } + + private static void validateHeaderName(String name) { + if (name == null) { + throw new NullPointerException("name"); + } + for (int i = 0; i < name.length(); i ++) { + char c = name.charAt(i); + if (c > 127) { + throw new IllegalArgumentException( + "name contains non-ascii character: " + name); + } + + // Check prohibited characters. + switch (c) { + case '=': case ',': case ';': case ' ': case ':': + case '\t': case '\r': case '\n': case '\f': + case 0x0b: // Vertical tab + throw new IllegalArgumentException( + "name contains one of the following prohibited characters: " + + "=,;: \\t\\r\\n\\v\\f: " + name); + } + } + } + + private static void validateHeaderValue(String value) { + if (value == null) { + throw new NullPointerException("value"); + } + for (int i = 0; i < value.length(); i ++) { + char c = value.charAt(i); + // Check prohibited characters. + switch (c) { + case '\r': case '\n': case '\f': + case 0x0b: // Vertical tab + throw new IllegalArgumentException( + "value contains one of the following prohibited characters: " + + "\\r\\n\\v\\f: " + value); + } } - headers.put(name, values); } public void removeHeader(final String name) { diff --git a/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java b/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java index a20587525e..8a04581ecb 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpMessage.java @@ -51,7 +51,7 @@ public interface HttpMessage { void setHeader(String name, String value); - void setHeader(String name, List values); + void setHeader(String name, Iterable values); void removeHeader(String name);