From 5a0e7866d387523275bdcc12d6ff9f101d8b26f7 Mon Sep 17 00:00:00 2001
From: Andy Green <andy.green@linaro.org>
Date: Thu, 21 Jan 2016 10:57:39 +0800
Subject: [PATCH] unify bounds checking in parser

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 lib/libwebsockets.h |  2 +-
 lib/parsers.c       | 52 +++++++++++++++++++++++++++++++++++-----------------
 lib/server.c        |  4 ++--
 3 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/lib/libwebsockets.h b/lib/libwebsockets.h
index 8ace40b..405d897 100644
--- a/lib/libwebsockets.h
+++ b/lib/libwebsockets.h
@@ -1449,7 +1449,7 @@ lws_add_http_header_status(struct lws *wsi,
 			   unsigned int code, unsigned char **p,
 			   unsigned char *end);
 
-LWS_VISIBLE LWS_EXTERN int
+LWS_VISIBLE LWS_EXTERN int LWS_WARN_UNUSED_RESULT
 lws_http_transaction_completed(struct lws *wsi);
 
 #ifdef LWS_USE_LIBEV
diff --git a/lib/parsers.c b/lib/parsers.c
index 3d198e1..5b7aecd 100644
--- a/lib/parsers.c
+++ b/lib/parsers.c
@@ -60,8 +60,8 @@ LWS_WARN_UNUSED_RESULT lextable_decode(int pos, char c)
 	}
 }
 
-int
-LWS_WARN_UNUSED_RESULT lws_allocate_header_table(struct lws *wsi)
+int LWS_WARN_UNUSED_RESULT
+lws_allocate_header_table(struct lws *wsi)
 {
 	struct lws_context *context = wsi->context;
 	int n;
@@ -275,8 +275,31 @@ char *lws_hdr_simple_ptr(struct lws *wsi, enum lws_token_indexes h)
 	return wsi->u.hdr.ah->data + wsi->u.hdr.ah->frags[n].offset;
 }
 
-int lws_hdr_simple_create(struct lws *wsi, enum lws_token_indexes h,
-			  const char *s)
+int LWS_WARN_UNUSED_RESULT
+lws_pos_in_bounds(struct lws *wsi)
+{
+	if (wsi->u.hdr.ah->pos < wsi->context->max_http_header_data)
+		return 0;
+
+	if (wsi->u.hdr.ah->pos == wsi->context->max_http_header_data) {
+		lwsl_err("Ran out of header data space\n");
+		return 1;
+	}
+
+	/*
+	 * with these tests everywhere, it should never be able to exceed
+	 * the limit, only meet the limit
+	 */
+
+	lwsl_err("%s: pos %d, limit %d\n", __func__, wsi->u.hdr.ah->pos,
+		 wsi->context->max_http_header_data);
+	assert(0);
+
+	return 1;
+}
+
+int LWS_WARN_UNUSED_RESULT
+lws_hdr_simple_create(struct lws *wsi, enum lws_token_indexes h, const char *s)
 {
 	wsi->u.hdr.ah->nfrag++;
 	if (wsi->u.hdr.ah->nfrag == ARRAY_SIZE(wsi->u.hdr.ah->frags)) {
@@ -291,10 +314,9 @@ int lws_hdr_simple_create(struct lws *wsi, enum lws_token_indexes h,
 	wsi->u.hdr.ah->frags[wsi->u.hdr.ah->nfrag].nfrag = 0;
 
 	do {
-		if (wsi->u.hdr.ah->pos == wsi->context->max_http_header_data) {
-			lwsl_err("Ran out of header data space\n");
+		if (lws_pos_in_bounds(wsi))
 			return -1;
-		}
+
 		wsi->u.hdr.ah->data[wsi->u.hdr.ah->pos++] = *s;
 		if (*s)
 			wsi->u.hdr.ah->frags[wsi->u.hdr.ah->nfrag].len++;
@@ -322,10 +344,8 @@ issue_char(struct lws *wsi, unsigned char c)
 {
 	unsigned short frag_len;
 
-	if (wsi->u.hdr.ah->pos == wsi->context->max_http_header_data) {
-		lwsl_warn("excessive header content\n");
+	if (lws_pos_in_bounds(wsi))
 		return -1;
-	}
 
 	frag_len = wsi->u.hdr.ah->frags[wsi->u.hdr.ah->nfrag].len;
 	/*
@@ -341,10 +361,8 @@ issue_char(struct lws *wsi, unsigned char c)
 
 	/* Insert a null character when we *hit* the limit: */
 	if (frag_len == wsi->u.hdr.current_token_limit) {
-		if (wsi->u.hdr.ah->pos == wsi->context->max_http_header_data) {
-			lwsl_warn("excessive header content 2\n");
+		if (lws_pos_in_bounds(wsi))
 			return -1;
-		}
 		wsi->u.hdr.ah->data[wsi->u.hdr.ah->pos++] = '\0';
 		lwsl_warn("header %i exceeds limit %d\n",
 			  wsi->u.hdr.parser_state, wsi->u.hdr.current_token_limit);
@@ -375,8 +393,8 @@ lws_parse(struct lws *wsi, unsigned char c)
 
 		/* collect into malloc'd buffers */
 		/* optional initial space swallow */
-		if (!ah->frags[ah->frag_index[
-				      wsi->u.hdr.parser_state]].len && c == ' ')
+		if (!ah->frags[ah->frag_index[wsi->u.hdr.parser_state]].len &&
+		    c == ' ')
 			break;
 
 		for (m = 0; m < ARRAY_SIZE(methods); m++)
@@ -674,7 +692,7 @@ excessive:
 
 		ah->frags[ah->nfrag].offset = ah->pos;
 		ah->frags[ah->nfrag].len = 0;
-		ah->frags[ ah->nfrag].nfrag = 0;
+		ah->frags[ah->nfrag].nfrag = 0;
 
 		n = ah->frag_index[wsi->u.hdr.parser_state];
 		if (!n) { /* first fragment */
@@ -683,7 +701,7 @@ excessive:
 		}
 		/* continuation */
 		while (ah->frags[n].nfrag)
-				n = ah->frags[n].nfrag;
+			n = ah->frags[n].nfrag;
 		ah->frags[n].nfrag = ah->nfrag;
 
 		if (issue_char(wsi, ' ') < 0)
diff --git a/lib/server.c b/lib/server.c
index 3be716d..036c876 100644
--- a/lib/server.c
+++ b/lib/server.c
@@ -626,8 +626,8 @@ lws_create_new_server_wsi(struct lws_context *context)
  *	  transaction if possible
  */
 
-LWS_VISIBLE
-int lws_http_transaction_completed(struct lws *wsi)
+LWS_VISIBLE int LWS_WARN_UNUSED_RESULT
+lws_http_transaction_completed(struct lws *wsi)
 {
 	lwsl_debug("%s: wsi %p\n", __func__, wsi);
 	/* if we can't go back to accept new headers, drop the connection */
-- 
2.7.4