Hi. Let me post a few more patches dealing with security. 1_of_4_Check_realloc_s_return_code_to_prevent_segfault_on_out_of_memory_condition__Part_2_.patch
This patch re-fixes the realloc problem from the previous patch. I forgot to restore the pointer to buffer in case of realloc error. 2_of_4_Replace_common__if__quiet__printf_______pattern_with_a_macro.patch This one replaces common if(!quiet) printf(...) pattern with a safe macro 3_of_4_Check_realloc_s_return_code_to_prevent_segfault_on_out_of_memory_condition__Part_3_.patch Here we have more realloc fixes, this time in http.c 4_of_4_Introduce_recv_timeout_controlled_by___T__option_in_http_c.patch The most important one: I found that http.c-based applications suffer from a kind of DDoS attacks where attacker opens connections to the application, but sends no data. As soon as all threads block in their [recv]s, application stops answering requests. This patch helps to protect the application by setting up a timeout for recv and an option to control it. Please, review/apply! Regards, Sergey
# HG changeset patch # User Sergey Mironov <[email protected]> # Date 1408867001 -14400 # Sun Aug 24 11:56:41 2014 +0400 # Node ID 76eaa0d4e25353afd042d69609cf1997851564c8 # Parent cbee668d155b788b45868ab4db3461c58a262547 Check realloc's return code to prevent segfault on out of memory condition (Part 2) diff --git a/src/c/request.c b/src/c/request.c --- a/src/c/request.c +++ b/src/c/request.c @@ -444,11 +444,12 @@ int len = strlen(inputs); if (len+1 > rc->queryString_size) { - rc->queryString = realloc(rc->queryString, len+1); - if(rc->queryString == NULL) { + char *qs = realloc(rc->queryString, len+1); + if(qs == NULL) { log_error(logger_data, "queryString is too long (not enough memory)\n"); return FAILED; } + rc->queryString = qs; rc->queryString_size = len+1; } strcpy(rc->queryString, inputs); @@ -485,11 +486,12 @@ on_success(ctx); if (path_len + 1 > rc->path_copy_size) { - rc->path_copy = realloc(rc->path_copy, path_len + 1); - if(rc->path_copy == NULL) { + char *pc = realloc(rc->path_copy, path_len + 1); + if(pc == NULL) { log_error(logger_data, "Path is too long (not enough memory)\n"); return FAILED; } + rc->path_copy = pc; rc->path_copy_size = path_len + 1; } strcpy(rc->path_copy, path);
# HG changeset patch # User Sergey Mironov <[email protected]> # Date 1409679374 0 # Tue Sep 02 17:36:14 2014 +0000 # Node ID 7961f5ddf6df82f403d0348faea795bbaa27157b # Parent 76eaa0d4e25353afd042d69609cf1997851564c8 Replace common "if(!quiet) printf(...)" pattern with a macro diff --git a/src/c/http.c b/src/c/http.c --- a/src/c/http.c +++ b/src/c/http.c @@ -23,6 +23,9 @@ int uw_backlog = SOMAXCONN; static int keepalive = 0, quiet = 0; +#define qfprintf(f, fmt, args...) do { if(!quiet) fprintf(f, fmt, ##args); } while(0) +#define qprintf(fmt, args...) do { if(!quiet) printf(fmt, ##args); } while(0) + static char *get_header(void *data, const char *h) { char *s = data; int len = strlen(h); @@ -86,8 +89,7 @@ sock = uw_dequeue(); } - if (!quiet) - printf("Handling connection with thread #%d.\n", me); + qprintf("Handling connection with thread #%d.\n", me); while (1) { int r; @@ -107,16 +109,14 @@ r = recv(sock, back, buf_size - 1 - (back - buf), 0); if (r < 0) { - if (!quiet) - fprintf(stderr, "Recv failed\n"); + qfprintf(stderr, "Recv failed while receiving header\n"); close(sock); sock = 0; break; } if (r == 0) { - if (!quiet) - printf("Connection closed.\n"); + qprintf("Connection closed.\n"); close(sock); sock = 0; break; @@ -159,16 +159,14 @@ r = recv(sock, back, buf_size - 1 - (back - buf), 0); if (r < 0) { - if (!quiet) - fprintf(stderr, "Recv failed\n"); + qfprintf(stderr, "Recv failed\n"); close(sock); sock = 0; goto done; } if (r == 0) { - if (!quiet) - fprintf(stderr, "Connection closed.\n"); + qfprintf(stderr, "Connection closed.\n"); close(sock); sock = 0; goto done; @@ -236,8 +234,7 @@ uw_set_headers(ctx, get_header, headers); uw_set_env(ctx, get_env, NULL); - if (!quiet) - printf("Serving URI %s....\n", path); + qprintf("Serving URI %s....\n", path); rr = uw_request(rc, ctx, method, path, query_string, body, back - body, on_success, on_failure, NULL, log_error, log_debug, @@ -411,8 +408,7 @@ sin_size = sizeof their_addr; - if (!quiet) - printf("Listening on port %d....\n", uw_port); + qprintf("Listening on port %d....\n", uw_port); { pthread_t thread; @@ -440,11 +436,9 @@ int new_fd = accept(sockfd, (struct sockaddr *)&their_addr, &sin_size); if (new_fd < 0) { - if (!quiet) - fprintf(stderr, "Socket accept failed\n"); + qfprintf(stderr, "Socket accept failed\n"); } else { - if (!quiet) - printf("Accepted connection.\n"); + qprintf("Accepted connection.\n"); if (keepalive) { int flag = 1;
# HG changeset patch # User Sergey Mironov <[email protected]> # Date 1409679442 0 # Tue Sep 02 17:37:22 2014 +0000 # Node ID 0ceae8e409b40a2cf9592aaeeb33326782a7783d # Parent 7961f5ddf6df82f403d0348faea795bbaa27157b Check realloc's return code to prevent segfault on out of memory condition (Part 3) diff --git a/src/c/http.c b/src/c/http.c --- a/src/c/http.c +++ b/src/c/http.c @@ -97,8 +97,15 @@ if (back - buf == buf_size - 1) { char *new_buf; - buf_size *= 2; - new_buf = realloc(buf, buf_size); + size_t new_buf_size = buf_size*2; + new_buf = realloc(buf, new_buf_size); + if(!new_buf) { + qfprintf(stderr, "Realloc failed while receiving header\n"); + close(sock); + sock = 0; + break; + } + buf_size = new_buf_size; back = new_buf + (back - buf); buf = new_buf; } @@ -146,9 +153,16 @@ while (back - body < clen) { if (back - buf == buf_size - 1) { char *new_buf; - buf_size *= 2; - new_buf = realloc(buf, buf_size); + size_t new_buf_size = buf_size * 2; + new_buf = realloc(buf, new_buf_size); + if(!new_buf) { + qfprintf(stderr, "Realloc failed while receiving content\n"); + close(sock); + sock = 0; + goto done; + } + buf_size = new_buf_size; back = new_buf + (back - buf); body = new_buf + (body - buf); s = new_buf + (s - buf);
# HG changeset patch # User Sergey Mironov <[email protected]> # Date 1409679730 0 # Tue Sep 02 17:42:10 2014 +0000 # Node ID a240354c2433a537c97208b2afff2b88799b21e8 # Parent 0ceae8e409b40a2cf9592aaeeb33326782a7783d Introduce recv timeout controlled by '-T' option in http.c This should prevent a DDoS attack where attacker and keeps the connection open but send no data. diff --git a/src/c/http.c b/src/c/http.c --- a/src/c/http.c +++ b/src/c/http.c @@ -116,7 +116,7 @@ r = recv(sock, back, buf_size - 1 - (back - buf), 0); if (r < 0) { - qfprintf(stderr, "Recv failed while receiving header\n"); + qfprintf(stderr, "Recv failed while receiving header, retcode %d errno %m\n", r); close(sock); sock = 0; break; @@ -173,7 +173,7 @@ r = recv(sock, back, buf_size - 1 - (back - buf), 0); if (r < 0) { - qfprintf(stderr, "Recv failed\n"); + qfprintf(stderr, "Recv failed while receiving content, retcode %d errno %m\n", r); close(sock); sock = 0; goto done; @@ -318,7 +318,7 @@ } static void help(char *cmd) { - printf("Usage: %s [-p <port>] [-a <IP address>] [-t <thread count>] [-k] [-q]\nThe '-k' option turns on HTTP keepalive.\nThe '-q' option turns off some chatter on stdout.\n", cmd); + printf("Usage: %s [-p <port>] [-a <IP address>] [-t <thread count>] [-k] [-q] [-T SEC]\nThe '-k' option turns on HTTP keepalive.\nThe '-q' option turns off some chatter on stdout.\nThe -T option sets socket recv timeout (0 disables timeout, default is 5 sec)", cmd); } static void sigint(int signum) { @@ -333,6 +333,7 @@ struct sockaddr_in their_addr; // connector's address information socklen_t sin_size; int yes = 1, uw_port = 8080, nthreads = 1, i, *names, opt; + int recv_timeout_sec = 5; signal(SIGINT, sigint); signal(SIGPIPE, SIG_IGN); @@ -340,7 +341,7 @@ my_addr.sin_addr.s_addr = INADDR_ANY; // auto-fill with my IP memset(my_addr.sin_zero, '\0', sizeof my_addr.sin_zero); - while ((opt = getopt(argc, argv, "hp:a:t:kq")) != -1) { + while ((opt = getopt(argc, argv, "hp:a:t:kqT:")) != -1) { switch (opt) { case '?': fprintf(stderr, "Unknown command-line option\n"); @@ -381,6 +382,15 @@ keepalive = 1; break; + case 'T': + recv_timeout_sec = atoi(optarg); + if (recv_timeout_sec < 0) { + fprintf(stderr, "Invalid recv timeout\n"); + help(argv[0]); + return 1; + } + break; + case 'q': quiet = 1; break; @@ -459,6 +469,17 @@ setsockopt(new_fd, IPPROTO_TCP, TCP_NODELAY, (char *) &flag, sizeof(int)); } + if(recv_timeout_sec>0) { + int ret; + struct timeval tv; + memset(&tv, 0, sizeof(struct timeval)); + tv.tv_sec = recv_timeout_sec; + ret = setsockopt(new_fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(struct timeval)); + if(ret != 0) { + qfprintf(stderr, "Timeout setting failed, errcode %d errno '%m'\n", ret); + } + } + uw_enqueue(new_fd); } }
_______________________________________________ Ur mailing list [email protected] http://www.impredicative.com/cgi-bin/mailman/listinfo/ur
