From 46c602c6db346e8fd4926fa57f7048851e728fc8 Mon Sep 17 00:00:00 2001 From: Thomas Ries Date: Fri, 13 Oct 2006 17:40:09 +0000 Subject: [PATCH] - Various fixes due to a code review by Andrew Jones --- ChangeLog | 5 +- configure.in | 1 + src/Makefile.am | 5 +- src/accessctl.c | 3 +- src/auth.c | 8 ++- src/plugin_shortdial.c | 2 +- src/proxy.c | 16 +++--- src/register.c | 19 +++++++ src/resolve.c | 2 +- src/rewrite_rules.h | 68 ------------------------ src/route_processing.c | 2 + src/sip_utils.c | 117 ++++++++++++----------------------------- src/siproxd.c | 5 +- src/siproxd.h | 3 +- src/utils.c | 2 +- 15 files changed, 88 insertions(+), 170 deletions(-) delete mode 100644 src/rewrite_rules.h diff --git a/ChangeLog b/ChangeLog index 599f664..b90280c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,9 +1,10 @@ 0.6.0 ===== - 05-Oct-2006_ - route_determine_nexthop(): don't remove header + 13-Oct-2006: - Various fixes due to a code review by Andrew Jones + 05-Oct-2006: - route_determine_nexthop(): don't remove header 19-Aug-2006: - increased DNS cache 18-Jun-2006: - included RTCP support and a de-jitter feature - (submitted by by Hans Carlos Hofmann) + (submitted by Hans Carlos Hofmann) 0.5.13 ====== diff --git a/configure.in b/configure.in index 76bf128..0c9a62a 100644 --- a/configure.in +++ b/configure.in @@ -399,6 +399,7 @@ fi AC_CHECK_FUNCS(getopt_long setsid syslog) AC_CHECK_FUNCS(getuid setuid getgid setgid getpwnam chroot) AC_CHECK_FUNCS(socket bind select read send sendto fcntl) +AC_CHECK_FUNCS(strcmp strcasecmp) AC_CHECK_FUNCS(strncpy strchr strstr sprintf vfprintf vsnprintf) AC_CHECK_FUNCS(listen accept) AC_CHECK_FUNCS(fgets sscanf) diff --git a/src/Makefile.am b/src/Makefile.am index 97d0fe3..d69ac17 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -31,6 +31,7 @@ siproxd_SOURCES = siproxd.c proxy.c register.c sock.c utils.c \ rtpproxy_relay.c accessctl.c route_processing.c \ security.c auth.c fwapi.c resolve.c \ plugin_shortdial.c +# addrcache.c # # an example for a custom firewall control module @@ -40,8 +41,8 @@ noinst_LIBRARIES = libcustom_fw_module.a libcustom_fw_module_a_SOURCES = custom_fw_module.c -noinst_HEADERS = log.h rewrite_rules.h siproxd.h digcalc.h rtpproxy.h \ - fwapi.h plugins.h +noinst_HEADERS = log.h siproxd.h digcalc.h rtpproxy.h \ + fwapi.h plugins.h addrcache.h EXTRA_DIST = .buildno diff --git a/src/accessctl.c b/src/accessctl.c index 8476367..da9a450 100644 --- a/src/accessctl.c +++ b/src/accessctl.c @@ -141,7 +141,7 @@ int process_aclist (char *aclist, struct sockaddr_in from) { /* mask */ p1=strchr(p2,','); - p1=p2+1; + p1=p1+1; p2=strchr(p1,','); if (!p2) { /* then this must be the last entry in the list */ p2=strchr(p1,'\0'); @@ -165,6 +165,7 @@ int process_aclist (char *aclist, struct sockaddr_in from) { } mask_int=atoi(mask); + if ((mask_int < 0) || (mask_int > 32)) mask_int=32; bitmask= (mask_int)? (0xffffffff<<(32-mask_int)) : 0; DEBUGC(DBCLASS_ACCESS,"check match: entry=%i, filter=%lx, from=%lx", i, diff --git a/src/auth.c b/src/auth.c index f5f6965..564f742 100644 --- a/src/auth.c +++ b/src/auth.c @@ -198,7 +198,11 @@ static int auth_check(osip_proxy_authorization_t *proxy_auth) { password=configuration.proxy_auth_passwd; } - if (password == NULL) password=""; + if (password == NULL) { + DEBUGC(DBCLASS_AUTH,"user [%s] not in password file!", + (Username)?Username:"*NULL*"); + return STS_FAILURE; + } DEBUGC(DBCLASS_BABBLE," username=\"%s\"",Username ); DEBUGC(DBCLASS_BABBLE," realm =\"%s\"",Realm ); @@ -281,7 +285,7 @@ static char *auth_getpwd(char *username) { /* strip comments and line with only whitespaces */ for (i=0;iusername[1])); - if (shortcut_no <= 0) return STS_SUCCESS; /* not a number */ + if ((shortcut_no <= 0) || (shortcut_no >= LONG_MAX)) return STS_SUCCESS; /* not a number */ /* requested number is not defined (out of range) */ if (shortcut_no > configuration.pi_shortdial_entry.used) { diff --git a/src/proxy.c b/src/proxy.c index c03aaf1..2b0926e 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -187,9 +187,8 @@ int proxy_request (sip_ticket_t *ticket) { * (rewrite request URI to point to the real host) */ /* 'i' still holds the valid index into the URLMAP table */ - if (check_rewrite_rq_uri(request) == STS_TRUE) { - proxy_rewrite_request_uri(request, i); - } + proxy_rewrite_request_uri(request, i); + /* if this is CANCEL/BYE request, stop RTP proxying */ if (MSG_IS_BYE(request) || MSG_IS_CANCEL(request)) { @@ -299,7 +298,7 @@ int proxy_request (sip_ticket_t *ticket) { { osip_header_t *max_forwards; int forwards_count = DEFAULT_MAXFWD; - char mfwd[8]; + char mfwd[12]; /* 10 digits, +/- sign, termination */ osip_message_get_max_forwards(request, 0, &max_forwards); if (max_forwards == NULL) { @@ -308,6 +307,8 @@ int proxy_request (sip_ticket_t *ticket) { } else { if (max_forwards->hvalue) { forwards_count = atoi(max_forwards->hvalue); + if ((forwards_count<=0)|| + (forwards_count>=LONG_MAX)) forwards_count=DEFAULT_MAXFWD; forwards_count -=1; osip_free (max_forwards->hvalue); } @@ -432,6 +433,7 @@ int proxy_request (sip_ticket_t *ticket) { if (url->port) { port=atoi(url->port); + if ((port<=0) || (port>65535)) port=SIP_PORT; } else { port=SIP_PORT; } @@ -750,6 +752,7 @@ int proxy_response (sip_ticket_t *ticket) { if (via->port) { port=atoi(via->port); + if ((port<=0) || (port>65535)) port=SIP_PORT; } else { port=SIP_PORT; } @@ -1000,8 +1003,7 @@ if (configuration.debuglevel) /* start an RTP proxying stream */ if (sdp_message_m_port_get(sdp, media_stream_no)) { msg_port=atoi(sdp_message_m_port_get(sdp, media_stream_no)); - - if (msg_port > 0) { + if ((msg_port > 0) && (msg_port <= 65535)) { /* is this an RTP stream ? */ { @@ -1044,7 +1046,7 @@ if (configuration.debuglevel) static char from_string[HOSTNAME_SIZE]; char *tmp; tmp=utils_inet_ntoa(ticket->from.sin_addr); - strncpy(from_string, tmp, HOSTNAME_SIZE-1); + strncpy(from_string, tmp, HOSTNAME_SIZE); from_string[HOSTNAME_SIZE-1]='\0'; client_id=from_string; } diff --git a/src/register.c b/src/register.c index 6611afb..08b1345 100644 --- a/src/register.c +++ b/src/register.c @@ -75,6 +75,7 @@ void register_init(void) { for (i=0;i < URLMAP_SIZE; i++) { fgets(buff, sizeof(buff), stream); sts=sscanf(buff, "***:%i:%i", &urlmap[i].active, &urlmap[i].expires); + if (sts == 0) break; /* format error */ if (urlmap[i].active) { osip_uri_init(&urlmap[i].true_url); osip_uri_init(&urlmap[i].masq_url); @@ -89,6 +90,7 @@ void register_init(void) { len = strlen(buff);\ X =(char*)malloc(len+1);\ sts=sscanf(buff,"%s",X);\ + if (sts == 0) break;\ } else {\ X = NULL;\ }\ @@ -109,6 +111,14 @@ void register_init(void) { } } fclose(stream); + + /* check for premature abort of reading the registration file */ + if (i < URLMAP_SIZE) { + /* clean up and delete it */ + WARN("registration file corrupt, starting with empty table"); + memset(urlmap, 0, sizeof(urlmap)); + unlink(configuration.registrationfile); + } } } /* initialize save-timer */ @@ -282,9 +292,13 @@ int register_client(sip_ticket_t *ticket, int force_lcl_masq) { if (expires_param && expires_param->gvalue) { /* get expires from contact Header */ expires=atoi(expires_param->gvalue); + if ((expires < 0) || (expires >= LONG_MAX)) + expires=configuration.default_expires; } else if (expires_hdr && expires_hdr->hvalue) { /* get expires from expires Header */ expires=atoi(expires_hdr->hvalue); + if ((expires < 0) || (expires >= LONG_MAX)) + expires=configuration.default_expires; } else { char tmp[16]; /* it seems, the expires field is not present everywhere... */ @@ -595,6 +609,7 @@ int register_response(sip_ticket_t *ticket, int flag) { /* send answer back */ if (via->port) { port=atoi(via->port); + if ((port<=0) || (port>65535)) port=SIP_PORT; } else { port=configuration.sip_listen_port; } @@ -650,9 +665,13 @@ int register_set_expire(sip_ticket_t *ticket) { if (expires_param && expires_param->gvalue) { /* get expires from contact Header */ expires=atoi(expires_param->gvalue); + if ((expires < 0) || (expires >= LONG_MAX)) + expires=configuration.default_expires; } else if (expires_hdr && expires_hdr->hvalue) { /* get expires from expires Header */ expires=atoi(expires_hdr->hvalue); + if ((expires < 0) || (expires >= LONG_MAX)) + expires=configuration.default_expires; } DEBUGC(DBCLASS_REG,"Expires=%i, expires_param=%p, expires_hdr=%p", diff --git a/src/resolve.c b/src/resolve.c index 3bb115c..788782c 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -209,7 +209,7 @@ static int _resolve(char *name, int proto, int type, priority = pr; weight = we; *port = po; - strncpy(dname, tmpname, dnamelen-1); + strncpy(dname, tmpname, dnamelen); xptr+=j; } } diff --git a/src/rewrite_rules.h b/src/rewrite_rules.h deleted file mode 100644 index dd02f45..0000000 --- a/src/rewrite_rules.h +++ /dev/null @@ -1,68 +0,0 @@ -/* - Copyright (C) 2002-2005 Thomas Ries - - This file is part of Siproxd. - - Siproxd is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - Siproxd is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with Siproxd; if not, write to the Free Software - Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -*/ - - -/* $Id$ */ - -/* - * SIP Method 'knowledg base' - * This table tells siproxd what incomming requests (which methods) - * it shell rewite and which not. E.g. kphone behaves silly - * if a incoming SUBSCRIBE request is rewritten to the local host... - */ -static struct { - char *UAstring; - int action[12]; -} RQ_rewrite[] = -{ -/* -1 means: rewrite, 0 means don't rewrite, -1 means default - UA string I A R B O I C R N S M P - N C E Y P N A E O U E R - V K G E T F N F T B S A - I I I O C E I S S C - T S O E R F C A K - E T N L Y R G -*/ -{"oSIP/Linphone",{-1, -1, -1, -1, -1, -1. -1, -1, -1, -1, -1, -1}}, -{"Windows RTC", {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1}}, -{"KPhone", {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1}}, -/* the following line holds the default entries */ -{NULL, { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}} -}; - -static struct { - char *name; - int size; -} RQ_method[] = { -{ "INVITE", 6 }, -{ "ACK", 3 }, -{ "REGISTER", 8 }, -{ "BYE", 3 }, -{ "OPTIONS", 7 }, -{ "INFO", 4 }, -{ "CANCEL", 6 }, -{ "REFER", 5 }, -{ "NOTIFY", 6 }, -{ "SUBSCRIBE", 9 }, -{ "MESSAGE", 7 }, -{ "PRACK", 5 }, -{ NULL, 0 } -}; diff --git a/src/route_processing.c b/src/route_processing.c index a67ce27..f73e889 100644 --- a/src/route_processing.c +++ b/src/route_processing.c @@ -83,6 +83,7 @@ int route_preprocess(sip_ticket_t *ticket){ */ url = osip_message_get_uri(mymsg); dest_port= (url->port)?atoi(url->port):SIP_PORT; + if ((dest_port <=0) || (dest_port >65535)) dest_port=SIP_PORT; if (get_ip_by_host(url->host, &addr1) == STS_SUCCESS) { if ((configuration.sip_listen_port == dest_port) && @@ -397,6 +398,7 @@ int route_determine_nexthop(sip_ticket_t *ticket, if (route->url->port) { *port=atoi(route->url->port); + if ((*port<=0) || (*port>65535)) *port=SIP_PORT; } else { *port=SIP_PORT; } diff --git a/src/sip_utils.c b/src/sip_utils.c index ec5fa59..8627780 100644 --- a/src/sip_utils.c +++ b/src/sip_utils.c @@ -40,7 +40,6 @@ #include "siproxd.h" #include "digcalc.h" -#include "rewrite_rules.h" #include "log.h" static char const ident[]="$Id$"; @@ -64,13 +63,6 @@ osip_message_t *msg_make_template_reply (sip_ticket_t *ticket, int code) { osip_message_t *response; int pos; - osip_message_init (&response); - response->message=NULL; - osip_message_set_version (response, osip_strdup ("SIP/2.0")); - osip_message_set_status_code (response, code); - osip_message_set_reason_phrase (response, - osip_strdup(osip_message_get_reason (code))); - if (request->to==NULL) { ERROR("msg_make_template_reply: empty To in request header"); return NULL; @@ -81,6 +73,18 @@ osip_message_t *msg_make_template_reply (sip_ticket_t *ticket, int code) { return NULL; } + osip_message_init (&response); + if (response == NULL) { + ERROR("msg_make_template_reply: osip_message_init() failed"); + return NULL; + } + response->message=NULL; + osip_message_set_version (response, osip_strdup ("SIP/2.0")); + osip_message_set_status_code (response, code); + osip_message_set_reason_phrase (response, + osip_strdup(osip_message_get_reason (code))); + + osip_to_clone (request->to, &response->to); osip_from_clone (request->from, &response->from); @@ -213,8 +217,12 @@ int is_via_local (osip_via_t *via) { } /* check the extracted VIA against my own host addresses */ - if (via->port) port=atoi(via->port); - else port=SIP_PORT; + if (via->port) { + port=atoi(via->port); + if ((port<=0) || (port>65535)) port=SIP_PORT; + } else { + port=SIP_PORT; + } if ( (memcmp(&addr_myself, &addr_via, sizeof(addr_myself))==0) && (port == configuration.sip_listen_port) ) { @@ -356,7 +364,7 @@ int compare_callid(osip_call_id_t *cid1, osip_call_id_t *cid2) { */ if (cid1->host && cid2->host) { /* have both hosts */ - if (strcmp(cid1->host, cid2->host) != 0) goto mismatch; + if (strcasecmp(cid1->host, cid2->host) != 0) goto mismatch; } else { /* at least one host missing, make sure that both are empty */ if ( (cid1->host && (cid1->host[0]!='\0')) || @@ -391,7 +399,7 @@ int is_sipuri_local (sip_ticket_t *ticket) { int found; struct in_addr addr_uri, addr_myself; int port; - int i; + int i, sts; if (sip==NULL) { ERROR("called is_sipuri_local with NULL sip"); @@ -409,7 +417,12 @@ int is_sipuri_local (sip_ticket_t *ticket) { if (utils_inet_aton(sip->req_uri->host, &addr_uri) == 0) { /* need name resolution */ - get_ip_by_host(sip->req_uri->host, &addr_uri); + sts=get_ip_by_host(sip->req_uri->host, &addr_uri); + if (sts == STS_FAILURE) { + DEBUGC(DBCLASS_PROXY, "sip_gen_response: cannot resolve via [%s]", + sip->req_uri->host); + return STS_FALSE; + } } found=0; @@ -426,6 +439,7 @@ int is_sipuri_local (sip_ticket_t *ticket) { /* check the extracted HOST against my own host addresses */ if (sip->req_uri->port) { port=atoi(sip->req_uri->port); + if ((port<=0) || (port>65535)) port=SIP_PORT; } else { port=SIP_PORT; } @@ -443,71 +457,6 @@ int is_sipuri_local (sip_ticket_t *ticket) { } -/* - * check if a given request (outbound -> inbound) shall its - * request URI get rewritten based upon our UA knowledge - * - * RETURNS - * STS_TRUE if to be rewritten - * STS_FALSE otherwise - */ -int check_rewrite_rq_uri (osip_message_t *sip) { - int i, j, sts; - int dflidx; - osip_header_t *ua_hdr; - - /* get index of default entry */ - dflidx=(sizeof(RQ_rewrite)/sizeof(RQ_rewrite[0])) - 1; - - /* check fort existence of method */ - if ((sip==NULL) || - (sip->sip_method==NULL)) { - ERROR("check_rewrite_rq_uri: got NULL method"); - return STS_FALSE; - } - - /* extract UA string */ - osip_message_get_user_agent (sip, 0, &ua_hdr); - if ((ua_hdr==NULL) || (ua_hdr->hvalue==NULL)) { - DEBUGC(DBCLASS_SIP, "check_rewrite_rq_uri: NULL UA in Header, " - "using default"); - i=dflidx; - } else { - /* loop through the knowledge base */ - for (i=0; RQ_rewrite[i].UAstring; i++) { - if (strncmp(RQ_rewrite[i].UAstring, ua_hdr->hvalue, - sizeof(RQ_rewrite[i].UAstring))==0) { - DEBUGC(DBCLASS_SIP, "got knowledge entry for [%s]", - ua_hdr->hvalue); - break; - } - } /* for i */ - } /* if ua_hdr */ - - for (j=0; RQ_method[j].name; j++) { - if (strncmp(RQ_method[j].name, - sip->sip_method, RQ_method[j].size)==0) { - if (RQ_rewrite[i].action[j] >= 0) { - sts = (RQ_rewrite[i].action[j])? STS_TRUE: STS_FALSE; - } else { - sts = (RQ_rewrite[dflidx].action[j])? STS_TRUE: STS_FALSE; - } - DEBUGC(DBCLASS_SIP, "check_rewrite_rq_uri: [%s:%s, i=%i, j=%i] " - "got action %s", - (sip && sip->sip_method) ? - sip->sip_method : "*NULL*", - (ua_hdr && ua_hdr->hvalue)? ua_hdr->hvalue:"*NULL*", - i, j, (sts==STS_TRUE)? "rewrite":"norewrite"); - return sts; - } - } /* for j */ - - WARN("check_rewrite_rq_uri: didn't get a hit of the method [%s]", - sip->sip_method); - return STS_FALSE; -} - - /* * SIP_GEN_RESPONSE * @@ -539,6 +488,7 @@ int sip_gen_response(sip_ticket_t *ticket, int code) { if (via == NULL) { ERROR("sip_gen_response: Cannot send response - no via field"); + osip_message_free(response); return STS_FAILURE; } @@ -552,6 +502,7 @@ int sip_gen_response(sip_ticket_t *ticket, int code) { if (sts == STS_FAILURE) { DEBUGC(DBCLASS_PROXY, "sip_gen_response: cannot resolve via [%s]", via->host); + osip_message_free(response); return STS_FAILURE; } } @@ -559,12 +510,14 @@ int sip_gen_response(sip_ticket_t *ticket, int code) { sts = sip_message_to_str(response, &buffer, &buflen); if (sts != 0) { ERROR("sip_gen_response: msg_2char failed"); + osip_message_free(response); return STS_FAILURE; } if (via->port) { port=atoi(via->port); + if ((port<=0) || (port>65535)) port=SIP_PORT; } else { port=SIP_PORT; } @@ -930,7 +883,7 @@ int sip_find_outbound_proxy(sip_ticket_t *ticket, struct in_addr *addr, * first match wins */ for (i=0; i65535)) *port=SIP_PORT; return STS_SUCCESS; @@ -1110,12 +1063,12 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { port_via=0; if (via->port) port_via=atoi(via->port); - if (port_via <= 0) port_via=SIP_PORT; + if ((port_via<=0) || (port_via>65535)) port_via=SIP_PORT; port_ua=0; if (urlmap[i].true_url->port) port_ua=atoi(urlmap[i].true_url->port); - if (port_ua <= 0) port_ua=SIP_PORT; + if ((port_ua<=0) || (port_ua>65535)) port_ua=SIP_PORT; DEBUGC(DBCLASS_SIP, "sip_find_direction: checking for registered " "host [%s:%i] <-> [%s:%i]", diff --git a/src/siproxd.c b/src/siproxd.c index 2a65621..3b1cd06 100644 --- a/src/siproxd.c +++ b/src/siproxd.c @@ -152,7 +152,7 @@ int main (int argc, char *argv[]) case 'c': /* load config file */ DEBUGC(DBCLASS_CONFIG,"option: config file=%s",optarg); i=sizeof(configfile)-1; - strncpy(configfile,optarg,i-1); + strncpy(configfile,optarg,i); configfile[i]='\0'; config_search=0; break; @@ -369,6 +369,8 @@ int main (int argc, char *argv[]) osip_message_get_max_forwards(ticket.sipmsg, 0, &max_forwards); if (max_forwards && max_forwards->hvalue) { forwards_count = atoi(max_forwards->hvalue); + if ((forwards_count<=0)|| + (forwards_count>=LONG_MAX)) forwards_count=DEFAULT_MAXFWD; } DEBUGC(DBCLASS_PROXY,"checking Max-Forwards (=%i)",forwards_count); @@ -455,6 +457,7 @@ int main (int argc, char *argv[]) url = osip_message_get_uri(ticket.sipmsg); dest_port= (url->port)?atoi(url->port):SIP_PORT; + if ((dest_port <=0) || (dest_port >65535)) dest_port=SIP_PORT; if ( (get_ip_by_host(url->host, &addr1) == STS_SUCCESS) && (get_interface_ip(IF_INBOUND,&addr2) == STS_SUCCESS) && diff --git a/src/siproxd.h b/src/siproxd.h index dd372b2..731f5e9 100644 --- a/src/siproxd.h +++ b/src/siproxd.h @@ -23,7 +23,7 @@ #ifdef DMALLOC #include #endif - +#include /* * table to hold the client registrations @@ -167,7 +167,6 @@ int is_via_local (osip_via_t *via); /*X*/ int compare_url(osip_uri_t *url1, osip_uri_t *url2); /*X*/ int compare_callid(osip_call_id_t *cid1, osip_call_id_t *cid2); /*X*/ int is_sipuri_local (sip_ticket_t *ticket); /*X*/ -int check_rewrite_rq_uri (osip_message_t *sip); /*X*/ int sip_gen_response(sip_ticket_t *ticket, int code); /*X*/ int sip_add_myvia (sip_ticket_t *ticket, int interface); /*X*/ int sip_del_myvia (sip_ticket_t *ticket); /*X*/ diff --git a/src/utils.c b/src/utils.c index beea7a6..18804bb 100644 --- a/src/utils.c +++ b/src/utils.c @@ -108,7 +108,7 @@ int get_ip_by_host(char *hostname, struct in_addr *addr) { */ for (i=0; i %s", hostname, utils_inet_ntoa(*addr));