security harden http parser a bit

Drop the connection during parsing for a few more cases that can't be legit.

Take care about trying to free rxflow_buffer only if we reached a connmode
where it exists

Change behaviour on setting unknown HTTP method to kill connection

Signed-off-by: Andy Green <andy.green@linaro.org>

diff --git a/lib/handshake.c b/lib/handshake.c
index b389194..0c2cecf 100644 (file)
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -105,6 +105,19 @@ libwebsocket_read(struct libwebsocket_context *context,
 
                if (!lws_hdr_total_length(wsi, WSI_TOKEN_UPGRADE) ||
                             !lws_hdr_total_length(wsi, WSI_TOKEN_CONNECTION)) {
+
+                       /* it's not websocket.... shall we accept it as http? */
+
+                       if (!lws_hdr_total_length(wsi, WSI_TOKEN_GET_URI)) {
+                               lwsl_warn("Missing URI in HTTP request\n");
+                               /* drop the header info */
+                               if (wsi->u.hdr.ah)
+                                       free(wsi->u.hdr.ah);
+                               goto bail;
+                       }
+
+                       lwsl_info("HTTP request for '%s'\n", lws_hdr_simple_ptr(wsi, WSI_TOKEN_GET_URI));
+
                        wsi->state = WSI_STATE_HTTP;
                        n = 0;
                        if (wsi->protocol->callback)

diff --git a/lib/libwebsockets.c b/lib/libwebsockets.c
index 6afec96..629f13b 100644 (file)
--- a/lib/libwebsockets.c
+++ b/lib/libwebsockets.c
@@ -343,9 +343,16 @@ just_kill_connection:
 
        if ((old_state == WSI_STATE_ESTABLISHED ||
             wsi->mode == LWS_CONNMODE_WS_SERVING ||
-            wsi->mode == LWS_CONNMODE_WS_CLIENT) && wsi->u.ws.rx_user_buffer) {
-               free(wsi->u.ws.rx_user_buffer);
-               wsi->u.ws.rx_user_buffer = NULL;
+            wsi->mode == LWS_CONNMODE_WS_CLIENT)) {
+
+               if (wsi->u.ws.rx_user_buffer) {
+                       free(wsi->u.ws.rx_user_buffer);
+                       wsi->u.ws.rx_user_buffer = NULL;
+               }
+               if (wsi->u.ws.rxflow_buffer) {
+                       free(wsi->u.ws.rxflow_buffer);
+                       wsi->u.ws.rxflow_buffer = NULL;
+               }
        }
 
        /* tell the user it's all over for this guy */
@@ -389,9 +396,6 @@ just_kill_connection:
        }
 #endif
 
-       if (wsi->u.ws.rxflow_buffer)
-               free(wsi->u.ws.rxflow_buffer);
-
 /*     lwsl_info("closing fd=%d\n", wsi->sock); */
 
 #ifdef LWS_OPENSSL_SUPPORT

diff --git a/lib/parsers.c b/lib/parsers.c
index 4202d3a..953e5d8 100644 (file)
--- a/lib/parsers.c
+++ b/lib/parsers.c
@@ -470,7 +470,13 @@ int libwebsocket_parse(struct libwebsocket *wsi, unsigned char c)
 
                if (wsi->u.hdr.name_buffer_pos ==
                                           sizeof(wsi->u.hdr.name_buffer) - 1) {
+                       /* did we see HTTP token yet? */
+                       if (!wsi->u.hdr.ah->frag_index[WSI_TOKEN_GET_URI]) {
+                               lwsl_info("junk before method\n");
+                               return -1;
+                       }
                        /* name bigger than we can handle, skip until next */
+                       wsi->u.hdr.name_buffer_pos = 0;
                        wsi->u.hdr.parser_state = WSI_TOKEN_SKIPPING;
                        break;
                }
@@ -490,14 +496,13 @@ int libwebsocket_parse(struct libwebsocket *wsi, unsigned char c)
                                wsi->u.hdr.parser_state = WSI_TOKEN_SKIPPING;
                                break;
                        }
-                       /* hm it's an unknown http method in fact */
-                       if (c == ' ') {
-                               lwsl_info("Unknown method %s\n",
-                                                       wsi->u.hdr.name_buffer);
-                               /* treat it as GET */
-                               wsi->u.hdr.parser_state = WSI_TOKEN_GET_URI;
-                               goto start_fragment;
-                       }
+                       /*
+                        * hm it's an unknown http method in fact,
+                        * treat as dangerous
+                        */
+
+                       lwsl_info("Unknown method - dropping\n");
+                       return -1;
                }
                if (lextable[wsi->u.hdr.lextable_pos + 1] == 0) {