File squid-3.0.PRE6-b9070_b9074_b9074_b9081_b9082_bnc525774.patch of Package squid-beta
diff -ruN ../squid-3.0.PRE6.orig/src/HttpMsg.cc ./src/HttpMsg.cc
--- ../squid-3.0.PRE6.orig/src/HttpMsg.cc 2007-04-29 00:26:37.000000000 +0200
+++ ./src/HttpMsg.cc 2010-03-12 15:50:59.000000000 +0100
@@ -150,12 +150,20 @@
buf->terminate(); // does not affect content size
// find the end of headers
- // TODO: Remove? httpReplyParseStep() should do similar checks
const size_t hdr_len = headersEnd(buf->content(), buf->contentSize());
+ // sanity check the start line to see if this is in fact an HTTP message
+ if (!sanityCheckStartLine(buf, hdr_len, error)) {
+ // NP: sanityCheck sets *error and sends debug warnings on syntax errors.
+ // if we have seen the connection close, this is an error too
+ if (eof && *error==HTTP_STATUS_NONE)
+ *error = HTTP_INVALID_HEADER;
+ return false;
+ }
+
+
if (hdr_len <= 0) {
- debugs(58, 3, "HttpMsg::parse: failed to find end of headers " <<
- "(eof: " << eof << ") in '" << buf->content() << "'");
+ debugs(58, 3, "HttpMsg::parse: failed to find end of headers (eof: " << eof << ") in '" << buf->content() << "'");
if (eof) // iff we have seen the end, this is an error
*error = HTTP_INVALID_HEADER;
@@ -164,37 +172,28 @@
}
// TODO: move to httpReplyParseStep()
- if (hdr_len > Config.maxReplyHeaderSize) {
- debugs(58, 1, "HttpMsg::parse: Too large reply header (" <<
- hdr_len << " > " << Config.maxReplyHeaderSize);
+ if (hdr_len > Config.maxReplyHeaderSize || (hdr_len <= 0 && (size_t)buf->contentSize() > Config.maxReplyHeaderSize)) {
+ debugs(58, 1, "HttpMsg::parse: Too large reply header (" << hdr_len << " > " << Config.maxReplyHeaderSize);
*error = HTTP_HEADER_TOO_LARGE;
return false;
}
- if (!sanityCheckStartLine(buf, error)) {
- debugs(58,1, HERE << "first line of HTTP message is invalid");
- *error = HTTP_INVALID_HEADER;
- return false;
- }
-
const int res = httpMsgParseStep(buf->content(), buf->contentSize(), eof);
if (res < 0) { // error
- debugs(58, 3, "HttpMsg::parse: cannot parse isolated headers " <<
- "in '" << buf->content() << "'");
+ debugs(58, 3, "HttpMsg::parse: cannot parse isolated headers in '" << buf->content() << "'");
*error = HTTP_INVALID_HEADER;
return false;
}
if (res == 0) {
- debugs(58, 2, "HttpMsg::parse: strange, need more data near '" <<
- buf->content() << "'");
+ debugs(58, 2, "HttpMsg::parse: strange, need more data near '" << buf->content() << "'");
+ *error = HTTP_INVALID_HEADER;
return false; // but this should not happen due to headersEnd() above
}
assert(res > 0);
- debugs(58, 9, "HttpMsg::parse success (" << hdr_len << " bytes) " <<
- "near '" << buf->content() << "'");
+ debugs(58, 9, "HttpMsg::parse success (" << hdr_len << " bytes) near '" << buf->content() << "'");
if (hdr_sz != (int)hdr_len) {
debugs(58, 1, "internal HttpMsg::parse vs. headersEnd error: " <<
@@ -371,9 +370,8 @@
packerClean(&p);
}
-HttpMsg *
-
// use HTTPMSGLOCK() instead of calling this directly
+HttpMsg *
HttpMsg::_lock()
{
lock_count++;
diff -ruN ../squid-3.0.PRE6.orig/src/HttpMsg.h ./src/HttpMsg.h
--- ../squid-3.0.PRE6.orig/src/HttpMsg.h 2007-04-06 06:50:04.000000000 +0200
+++ ./src/HttpMsg.h 2010-03-12 15:36:42.000000000 +0100
@@ -91,7 +91,15 @@
void firstLineBuf(MemBuf&);
protected:
- virtual bool sanityCheckStartLine(MemBuf *buf, http_status *error) = 0;
+ /**
+ * Validate the message start line is syntactically correct.
+ * Set HTTP error status according to problems found.
+ *
+ * \retval true Status line has no serious problems.
+ * \retval false Status line has a serious problem. Correct response is indicated by error.
+ */
+ virtual bool sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status *error) = 0;
+
virtual void packFirstLineInto(Packer * p, bool full_uri) const = 0;
diff -ruN ../squid-3.0.PRE6.orig/src/HttpReply.cc ./src/HttpReply.cc
--- ../squid-3.0.PRE6.orig/src/HttpReply.cc 2007-04-20 09:29:47.000000000 +0200
+++ ./src/HttpReply.cc 2010-03-12 15:48:56.000000000 +0100
@@ -433,14 +433,55 @@
return content_length;
}
-bool HttpReply::sanityCheckStartLine(MemBuf *buf, http_status *error)
+
+/**
+ * Checks the first line of an HTTP Reply is valid.
+ * currently only checks "HTTP/" exists.
+ *
+ * NP: not all error cases are detected yet. Some are left for detection later in parse.
+ */
+bool
+HttpReply::sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status *error)
{
- if (buf->contentSize() >= protoPrefix.size() && protoPrefix.cmp(buf->content(), protoPrefix.size()) != 0) {
+ // content is long enough to possibly hold a reply
+ // 4 being magic size of a 3-digit number plus space delimiter
+ if ( buf->contentSize() < (protoPrefix.size() + 4) ) {
+ if (hdr_len > 0) {
+ debugs(58, 3, HERE << "Too small reply header (" << hdr_len << " bytes)");
+ *error = HTTP_INVALID_HEADER;
+ }
+ return false;
+ }
+
+ // catch missing or mismatched protocol identifier
+ if (protoPrefix.cmp(buf->content(), protoPrefix.size()) != 0) {
+
debugs(58, 3, "HttpReply::sanityCheckStartLine: missing protocol prefix (" << protoPrefix.buf() << ") in '" << buf->content() << "'");
*error = HTTP_INVALID_HEADER;
return false;
}
+ // catch missing or negative status value (negative '-' is not a digit)
+ int pos = protoPrefix.size();
+
+ // skip arbitrary number of digits and a dot in the verion portion
+ while ( pos <= buf->contentSize() && (*(buf->content()+pos) == '.' || xisdigit(*(buf->content()+pos)) ) ) ++pos;
+
+ // catch missing version info
+ if (pos == protoPrefix.size()) {
+ debugs(58, 3, "HttpReply::sanityCheckStartLine: missing protocol version numbers (ie. " << protoPrefix << "/1.0) in '" << buf->content() << "'");
+ *error = HTTP_INVALID_HEADER;
+ return false;
+ }
+
+ // skip arbitrary number of spaces...
+ while (pos <= buf->contentSize() && (char)*(buf->content()+pos) == ' ') ++pos;
+
+ if (!xisdigit(*(buf->content()+pos))) {
+ debugs(58, 3, "HttpReply::sanityCheckStartLine: missing or invalid status number in '" << buf->content() << "'");
+ *error = HTTP_INVALID_HEADER;
+ return false;
+ }
return true;
}
diff -ruN ../squid-3.0.PRE6.orig/src/HttpReply.h ./src/HttpReply.h
--- ../squid-3.0.PRE6.orig/src/HttpReply.h 2006-04-22 07:29:18.000000000 +0200
+++ ./src/HttpReply.h 2010-03-12 15:39:32.000000000 +0100
@@ -66,9 +66,9 @@
//virtual void unlock(); // only needed for debugging
// returns true on success
- // returns false and sets *error to zero when needs more data
+ // returns false and leaves *error unchanged when needs more data
// returns false and sets *error to a positive http_status code on error
- virtual bool sanityCheckStartLine(MemBuf *buf, http_status *error);
+ virtual bool sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status *error);
/* public, readable; never update these or their .hdr equivalents directly */
time_t date;
diff -ruN ../squid-3.0.PRE6.orig/src/HttpRequest.cc ./src/HttpRequest.cc
--- ../squid-3.0.PRE6.orig/src/HttpRequest.cc 2007-05-09 11:07:38.000000000 +0200
+++ ./src/HttpRequest.cc 2010-03-12 15:49:35.000000000 +0100
@@ -142,17 +142,31 @@
init();
}
+/**
+ * Checks the first line of an HTTP request is valid
+ * currently just checks the request method is present.
+ *
+ * NP: Other errors are left for detection later in the parse.
+ */
bool
-HttpRequest::sanityCheckStartLine(MemBuf *buf, http_status *error)
+HttpRequest::sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status *error)
{
- /*
- * Just see if the request buffer starts with a known
- * HTTP request method. NOTE this whole function is somewhat
- * superfluous and could just go away.
- */
+ // content is long enough to possibly hold a reply
+ // 2 being magic size of a 1-byte request method plus space delimiter
+ if ( buf->contentSize() < 2 ) {
+ // this is ony a real error if the headers apparently complete.
+ if (hdr_len > 0) {
+ debugs(58, 3, HERE << "Too large request header (" << hdr_len << " bytes)");
+ *error = HTTP_INVALID_HEADER;
+ }
+ return false;
+ }
+
+ /* See if the request buffer starts with a known HTTP request method. */
if (METHOD_NONE == HttpRequestMethod(buf->content())) {
debugs(73, 3, "HttpRequest::sanityCheckStartLine: did not find HTTP request method");
+ *error = HTTP_INVALID_HEADER;
return false;
}
diff -ruN ../squid-3.0.PRE6.orig/src/HttpRequest.h ./src/HttpRequest.h
--- ../squid-3.0.PRE6.orig/src/HttpRequest.h 2007-05-09 09:36:24.000000000 +0200
+++ ./src/HttpRequest.h 2010-03-12 15:42:09.000000000 +0100
@@ -157,7 +157,8 @@
protected:
virtual void packFirstLineInto(Packer * p, bool full_uri) const;
- virtual bool sanityCheckStartLine(MemBuf *buf, http_status *error);
+ virtual bool sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status *error);
+
virtual void hdrCacheInit();
diff -ruN ../squid-3.0.PRE6.orig/src/client_side.cc ./src/client_side.cc
--- ../squid-3.0.PRE6.orig/src/client_side.cc 2007-05-09 11:07:38.000000000 +0200
+++ ./src/client_side.cc 2010-03-12 15:25:26.000000000 +0100
@@ -1831,6 +1831,17 @@
/* pre-set these values to make aborting simpler */
*method_p = METHOD_NONE;
+ /* NP: don't be tempted to move this down or remove again.
+ * It's the only DDoS protection old-String has against long URL */
+ if ( hp->bufsiz <= 0) {
+ debugs(33, 5, "Incomplete request, waiting for end of request line");
+ return NULL;
+ }
+ else if ( (size_t)hp->bufsiz >= Config.maxRequestHeaderSize && headersEnd(hp->buf, Config.maxRequestHeaderSize) == 0) {
+ debugs(33, 5, "parseHttpRequest: Too large request");
+ return parseHttpRequestAbort(conn, "error:request-too-large");
+ }
+
/* Attempt to parse the first line; this'll define the method, url, version and header begin */
r = HttpParserParseReqLine(hp);
diff -ruN ../squid-3.0.PRE6.orig/src/http.cc ./src/http.cc
--- ../squid-3.0.PRE6.orig/src/http.cc 2007-05-08 18:37:59.000000000 +0200
+++ ./src/http.cc 2010-03-12 15:25:26.000000000 +0100
@@ -82,7 +82,7 @@
surrogateNoStore = false;
fd = fwd->server_fd;
readBuf = new MemBuf;
- readBuf->init(4096, SQUID_TCP_SO_RCVBUF);
+ readBuf->init();
orig_request = HTTPMSGLOCK(fwd->request);
if (fwd->servers)
diff -ruN ../squid-3.0.PRE6.orig/src/tests/stub_HttpReply.cc ./src/tests/stub_HttpReply.cc
--- ../squid-3.0.PRE6.orig/src/tests/stub_HttpReply.cc 2006-05-27 02:35:05.000000000 +0200
+++ ./src/tests/stub_HttpReply.cc 2010-03-12 15:43:30.000000000 +0100
@@ -76,7 +76,7 @@
}
bool
-HttpReply::sanityCheckStartLine(MemBuf *buf, http_status *error)
+HttpReply::sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status *error)
{
fatal ("Not implemented");
return false;
diff -ruN ../squid-3.0.PRE6.orig/src/tests/stub_HttpRequest.cc ./src/tests/stub_HttpRequest.cc
--- ../squid-3.0.PRE6.orig/src/tests/stub_HttpRequest.cc 2006-04-18 14:46:13.000000000 +0200
+++ ./src/tests/stub_HttpRequest.cc 2010-03-12 15:44:12.000000000 +0100
@@ -56,7 +56,7 @@
}
bool
-HttpRequest::sanityCheckStartLine(MemBuf *buf, http_status *error)
+HttpRequest::sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status *error)
{
fatal("Not implemented");
return false;
