Browse Source

[#1276] limit preamble, add test case

tim 10 months ago
parent
commit
c8bdc706b6
2 changed files with 89 additions and 5 deletions
  1. 9 5
      src/handle_form.inl
  2. 80 0
      unittest/public_server.c

+ 9 - 5
src/handle_form.inl

@@ -716,9 +716,11 @@ mg_handle_form_request(struct mg_connection *conn,
 
 
 			if (part_no == 0) {
 			if (part_no == 0) {
 				size_t preamble_length = 0;
 				size_t preamble_length = 0;
-				/* skip over the preamble until we find a complete boundary */
+				/* skip over the preamble until we find a complete boundary
+				 * limit the preamble length to prevent abuse */
 				/* +2 for the -- preceding the boundary */
 				/* +2 for the -- preceding the boundary */
-				while ((preamble_length < buf_fill - bl)
+				while (preamble_length < 1024
+				       && (preamble_length < buf_fill - bl)
 				       && strncmp(buf + preamble_length + 2, boundary, bl)) {
 				       && strncmp(buf + preamble_length + 2, boundary, bl)) {
 					preamble_length++;
 					preamble_length++;
 				}
 				}
@@ -732,9 +734,11 @@ mg_handle_form_request(struct mg_connection *conn,
 				}
 				}
 			}
 			}
 
 
-			/* either it starts with a boundary
-			 * or we couldn't find a boundary at all in the body
-			 * or we didn't have a terminating boundary */
+			/* either it starts with a boundary and it's fine, or it's malformed
+			 * because:
+			 * - the preamble was longer than accepted
+			 * - couldn't find a boundary at all in the body
+			 * - didn't have a terminating boundary */
 			if (buf_fill < (bl + 2) || strncmp(buf, "--", 2)
 			if (buf_fill < (bl + 2) || strncmp(buf, "--", 2)
 			    || strncmp(buf + 2, boundary, bl)) {
 			    || strncmp(buf + 2, boundary, bl)) {
 				/* Malformed request */
 				/* Malformed request */

+ 80 - 0
unittest/public_server.c

@@ -3766,6 +3766,86 @@ START_TEST(test_handle_form)
 	ck_assert_int_eq(client_ri->status_code, 200);
 	ck_assert_int_eq(client_ri->status_code, 200);
 	mg_close_connection(client_conn);
 	mg_close_connection(client_conn);
 
 
+	/* Handle form: "POST multipart/form-data" very long preamble */
+	multipart_body =
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "preamblepreamblepreamblepreamblepreamble\r\n"
+	    "--multipart-form-data-boundary--see-RFC-2388\r\n";
+	    "Content-Disposition: form-data; name=\"passwordin\"\r\n"
+	    "\r\n"
+	    "\r\n"
+	    "--multipart-form-data-boundary--see-RFC-2388--\r\n";
+
+	body_len = strlen(multipart_body);
+	ck_assert_uint_eq(body_len, 1768); /* not required */
+
+	client_conn =
+	    mg_download("localhost",
+	                8884,
+	                0,
+	                ebuf,
+	                sizeof(ebuf),
+	                "POST /handle_form_error HTTP/1.1\r\n"
+	                "Host: localhost:8884\r\n"
+	                "Connection: close\r\n"
+	                "Content-Type: multipart/form-data; "
+	                "boundary=multipart-form-data-boundary--see-RFC-2388\r\n"
+	                "Content-Length: %u\r\n"
+	                "\r\n%s",
+	                (unsigned int)body_len,
+	                multipart_body);
+
+	ck_assert(client_conn != NULL);
+	for (sleep_cnt = 0; sleep_cnt < 30; sleep_cnt++) {
+		test_sleep(1);
+		if (g_field_step == 1000) {
+			break;
+		}
+	}
+	client_ri = mg_get_response_info(client_conn);
+
+	ck_assert(client_ri != NULL);
+	ck_assert_int_eq(client_ri->status_code, 200);
+	mg_close_connection(client_conn);
 
 
 	/* Now test form_store */
 	/* Now test form_store */