
perki_pat3 at yahoo
Apr 23, 2008, 7:33 AM
Post #3 of 5
(161 views)
Permalink
|
|
Re: issue 43: Problem in the range port cherokee-admin
[In reply to]
|
|
Perki Pat escribi贸: > Alvaro Lopez Ortega escribi贸: >> On 23 Apr 2008, at 01:59, Perki Pat wrote: >> The patch is good, congrats! However, there is a couple of changes >> that would improve the patch before we apply it: >> >> - I would move the checking code to the initialize_server_socket() >> function in cherokee/server.c file. In that way the port would always >> be checked, not only when the -p parameter is used; for instance, it >> would ensure that a port read from a configuration file is correct as >> well. > > initialize_server_socket() receives an unsigned short argument for the > port: it's always in range (gcc told me). > > It's called from a couple places (server.c:940 and server.c:1070), and > in both places it takes its value from a cherokee_server_t struct, which > has a couple field (port and port_tls) which are also unsigned short. > > I'm looking where that fields are assigned. I've found where they're > filled from config file. Now I'll look for more places. ok, I think this choice is done. I'll paste the diffs below. We can check that if I'm root the port or port_tls is greater than 1024. (Windows also?) Or maybe it's better to "outsource" it to bind()+perror()?? I've changed severals cuint_t port; with cushort_t port; I've seen several atoi()s with no boundaries checking (server.c:configure_server_property(), lines circa 1480). is right? Also, when I do cherokee -p 67, if the config file says server!port=81, cherokee listens in port 81. Maybe command line arguments should "overwrite" config file statements. In the practicals I do at the university I'm used to always assert pointer arguments received by a function: #include <assert.h> /*and define DEBUG elsewhere*/ /*...*/ static ret_t configure_server_property (cherokee_config_node_t *conf, void *data) { /*variable definitions*/ assert(conf && data); /*real work...*/ The problem with paranoical error checking is that a 200 lines program becomes a 700 one, but maybe it helps hunting several bugs. ps: cc to list? pss: the diffs: pablo[at]golgi:~$ for i in cherokee-old/cherokee/*.[ch]; do diff -u $i `echo $i | sed "s/cherokee-old/cherokee-new/" -`; done; --- cherokee-old/cherokee/main_admin.c 2008-04-23 13:23:06.000000000 +0200 +++ cherokee-new/cherokee/main_admin.c 2008-04-23 13:29:28.000000000 +0200 @@ -46,7 +46,7 @@ #define DEFAULT_BIND "127.0.0.1" #define RULE "vserver!default!rule!" -static int port = DEFAULT_PORT; +static cushort_t port = DEFAULT_PORT; static char *document_root = DEFAULT_DOCUMENTROOT; static char *config_file = DEFAULT_CONFIG_FILE; static char *bind_to = DEFAULT_BIND; @@ -121,7 +121,7 @@ static void process_parameters (int argc, char **argv) { - int c; + int c,tmp_port; struct option long_options[] = { {"help", no_argument, NULL, 'h'}, @@ -142,7 +142,12 @@ bind_to = NULL; break; case 'p': - port = atoi(optarg); + tmp_port = atoi(optarg); + if(tmp_port<1 || tmp_port>65535) { + PRINT_MSG("Port %d is not between 1 and 65535\n",tmp_port); + exit(1); + } + port=tmp_port; break; case 'd': document_root = strdup(optarg); --- cherokee-old/cherokee/main.c 2008-04-23 13:23:06.000000000 +0200 +++ cherokee-new/cherokee/main.c 2008-04-23 12:03:03.000000000 +0200 @@ -69,7 +69,7 @@ static char *document_root = NULL; static cherokee_boolean_t daemon_mode = false; static cherokee_boolean_t just_test = false; -static cuint_t port = 80; +static cushort_t port = 80; static ret_t common_server_initialization (cherokee_server_t *srv); @@ -202,7 +202,7 @@ static void process_parameters (int argc, char **argv) { - int c; + int c,tmp_port; struct option long_options[] = { {"help", no_argument, NULL, 'h'}, @@ -227,7 +227,12 @@ document_root = strdup(optarg); break; case 'p': - port = atoi(optarg); + tmp_port = atoi(optarg); + if(tmp_port<1 || tmp_port>65535) { + PRINT_MSG("Port %d is not between 1 and 65535\n",tmp_port); + exit(1); + } + port=tmp_port; break; case 't': just_test = true; --- cherokee-old/cherokee/server.c 2008-04-23 13:23:06.000000000 +0200 +++ cherokee-new/cherokee/server.c 2008-04-23 13:00:02.000000000 +0200 @@ -1462,13 +1462,22 @@ ret_t ret; char *key = conf->key.buf; cherokee_server_t *srv = SRV(data); + int tmp; if (equal_buf_str (&conf->key, "port")) { - srv->port = atoi(conf->val.buf); - + tmp = atoi(conf->val.buf); + if(tmp<1 || tmp>65535) { + PRINT_MSG("Port %d is not between 1 and 65535\n",tmp); + return ret_error; + } + srv->port=tmp; } else if (equal_buf_str (&conf->key, "port_tls")) { - srv->port_tls = atoi(conf->val.buf); - + tmp = atoi(conf->val.buf); + if(tmp<1 || tmp>65535) { + PRINT_MSG("TLS port %d is not between 1 and 65535\n",tmp); + return ret_error; + } + srv->port_tls=tmp; } else if (equal_buf_str (&conf->key, "fdlimit")) { srv->fdlimit_custom = atoi (conf->val.buf); ______________________________________________ LLama Gratis a cualquier PC del Mundo. Llamadas a fijos y m髒iles desde 1 c閚timo por minuto. http://es.voice.yahoo.com
|