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:
[EMAIL PROTECTED]:~$ 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�viles desde 1 c�ntimo por minuto.
http://es.voice.yahoo.com
_______________________________________________
Cherokee mailing list
[email protected]
http://cherokee-project.com/cgi-bin/mailman/listinfo/cherokee
