[asterisk-commits] mmichelson: trunk r169557 - in /trunk: ./ channels/chan_sip.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Jan 20 14:10:31 CST 2009


Author: mmichelson
Date: Tue Jan 20 14:10:31 2009
New Revision: 169557

URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=169557
Log:
Convert the character pointers in a sip_request to be pointer offsets

When an ast_str expands to hold more data, any pointers that were pointing
to the data prior to the expansion will be pointing at invalid memory. This
change makes such pointers used in chan_sip.c instead be offsets from the
beginning of the string so that the same math may be applied no matter where
in memory the string resides.

To help ease this transition, a macro called REQ_OFFSET_TO_STR has been added
to chan_sip.c so that given a sip_request and an offset, the string at that
offset is returned.

(closes issue #14220)
Reported by: riksta
Tested by: putnopvut

Review http://reviewboard.digium.com/r/126/


Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c

Propchange: trunk/
------------------------------------------------------------------------------
    automerge = *

Propchange: trunk/
------------------------------------------------------------------------------
    automerge-email = mmichelson at digium.com

Propchange: trunk/
------------------------------------------------------------------------------
    svnmerge-integrated = /trunk:1-169442

Modified: trunk/channels/chan_sip.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/channels/chan_sip.c?view=diff&rev=169557&r1=169556&r2=169557
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Tue Jan 20 14:10:31 2009
@@ -1187,8 +1187,8 @@
  * \endverbatim
  */
 struct sip_request {
-	char *rlPart1; 	        /*!< SIP Method Name or "SIP/2.0" protocol version */
-	char *rlPart2; 	        /*!< The Request URI or Response Status */
+	ptrdiff_t rlPart1; 	        /*!< Offset of the SIP Method Name or "SIP/2.0" protocol version */
+	ptrdiff_t rlPart2; 	        /*!< Offset of the Request URI or Response Status */
 	int len;                /*!< bytes used in data[], excluding trailing null terminator. Rarely used. */
 	int headers;            /*!< # of SIP Headers */
 	int method;             /*!< Method of this request */
@@ -1198,13 +1198,24 @@
 	char debug;		/*!< print extra debugging if non zero */
 	char has_to_tag;	/*!< non-zero if packet has To: tag */
 	char ignore;		/*!< if non-zero This is a re-transmit, ignore it */
-	char *header[SIP_MAX_HEADERS];
-	char *line[SIP_MAX_LINES];
+	/* Array of offsets into the request string of each SIP header*/
+	ptrdiff_t header[SIP_MAX_HEADERS];
+	/* Array of offsets into the request string of each SDP line*/
+	ptrdiff_t line[SIP_MAX_LINES];
 	struct ast_str *data;	
 	/* XXX Do we need to unref socket.ser when the request goes away? */
 	struct sip_socket socket;	/*!< The socket used for this request */
 	AST_LIST_ENTRY(sip_request) next;
 };
+
+/* \brief given a sip_request and an offset, return the char * that resides there
+ *
+ * It used to be that rlPart1, rlPart2, and the header and line arrays were character
+ * pointers. They are now offsets into the ast_str portion of the sip_request structure.
+ * To avoid adding a bunch of redundant pointer arithmetic to the code, this macro is
+ * provided to retrieve the string at a particular offset within the request's buffer
+ */
+#define REQ_OFFSET_TO_STR(req,offset) (ast_str_buffer((req)->data) + ((req)->offset))
 
 /*! \brief structure used in transfers */
 struct sip_dual {
@@ -2745,7 +2756,7 @@
 		}
 
 		/* Read in headers one line at a time */
-		while (req.len < 4 || strncmp((char *)&req.data->str + req.len - 4, "\r\n\r\n", 4)) {
+		while (req.len < 4 || strncmp(REQ_OFFSET_TO_STR(&req, len - 4), "\r\n\r\n", 4)) {
 			ast_mutex_lock(&tcptls_session->lock);
 			if (!fgets(buf, sizeof(buf), tcptls_session->f)) {
 				ast_mutex_unlock(&tcptls_session->lock);
@@ -3758,7 +3769,7 @@
 	if (!req->lines) {
 		/* Add extra empty return. add_header() reserves 4 bytes so cannot be truncated */
 		ast_str_append(&req->data, 0, "\r\n");
-		req->len = req->data->used;
+		req->len = ast_str_strlen(req->data);
 	}
 }
 
@@ -3777,10 +3788,10 @@
 			ntohs(dst->sin_port), req->data->str);
 	}
 	if (p->do_history) {
-		struct sip_request tmp = { .rlPart1 = NULL, };
+		struct sip_request tmp = { .rlPart1 = 0, };
 		parse_copy(&tmp, req);
 		append_history(p, reliable ? "TxRespRel" : "TxResp", "%s / %s - %s", tmp.data->str, get_header(&tmp, "CSeq"), 
-			(tmp.method == SIP_RESPONSE || tmp.method == SIP_UNKNOWN) ? tmp.rlPart2 : sip_methods[tmp.method].text);
+			(tmp.method == SIP_RESPONSE || tmp.method == SIP_UNKNOWN) ? REQ_OFFSET_TO_STR(&tmp, rlPart2) : sip_methods[tmp.method].text);
 		ast_free(tmp.data);
 	}
 	res = (reliable) ?
@@ -3813,7 +3824,7 @@
 			ast_verbose("%sTransmitting (no NAT) to %s:%d:\n%s\n---\n", reliable ? "Reliably " : "", ast_inet_ntoa(p->sa.sin_addr), ntohs(p->sa.sin_port), req->data->str);
 	}
 	if (p->do_history) {
-		struct sip_request tmp = { .rlPart1 = NULL, };
+		struct sip_request tmp = { .rlPart1 = 0, };
 		parse_copy(&tmp, req);
 		append_history(p, reliable ? "TxReqRel" : "TxReq", "%s / %s - %s", tmp.data->str, get_header(&tmp, "CSeq"), sip_methods[tmp.method].text);
 		ast_free(tmp.data);
@@ -6241,7 +6252,7 @@
 	int len = strlen(name);
 
 	while (*start < req->sdp_end) {
-		const char *r = get_body_by_line(req->line[(*start)++], name, len, '=');
+		const char *r = get_body_by_line(REQ_OFFSET_TO_STR(req, line[(*start)++]), name, len, '=');
 		if (r[0] != '\0')
 			return r;
 	}
@@ -6265,7 +6276,7 @@
 	char *r;
 
 	for (x = 0; x < req->lines; x++) {
-		r = get_body_by_line(req->line[x], name, len, delimiter);
+		r = get_body_by_line(REQ_OFFSET_TO_STR(req, line[x]), name, len, delimiter);
 		if (r[0] != '\0')
 			return r;
 	}
@@ -6327,9 +6338,10 @@
 	 */
 	for (pass = 0; name && pass < 2;pass++) {
 		int x, len = strlen(name);
-		for (x=*start; x<req->headers; x++) {
-			if (!strncasecmp(req->header[x], name, len)) {
-				char *r = req->header[x] + len;	/* skip name */
+		for (x = *start; x < req->headers; x++) {
+			char *header = REQ_OFFSET_TO_STR(req, header[x]);
+			if (!strncasecmp(header, name, len)) {
+				char *r = header + len;	/* skip name */
 				if (sip_cfg.pedanticsipchecking)
 					r = ast_skip_blanks(r);
 
@@ -7074,33 +7086,38 @@
 */
 static int parse_request(struct sip_request *req)
 {
-	char *c = req->data->str, **dst = req->header;
+	char *c = req->data->str;
+	ptrdiff_t *dst = req->header;
 	int i = 0, lim = SIP_MAX_HEADERS - 1;
 	unsigned int skipping_headers = 0;
-
-	req->header[0] = c;
+	ptrdiff_t current_header_offset = 0;
+	char *previous_header = "";
+
+	req->header[0] = 0;
 	req->headers = -1;	/* mark that we are working on the header */
 	for (; *c; c++) {
 		if (*c == '\r') {		/* remove \r */
 			*c = '\0';
 		} else if (*c == '\n') { 	/* end of this line */
 			*c = '\0';
+			current_header_offset = (c + 1) - req->data->str;
+			previous_header = req->data->str + dst[i];
 			if (skipping_headers) {
 				/* check to see if this line is blank; if so, turn off
 				   the skipping flag, so the next line will be processed
 				   as a body line */
-				if (ast_strlen_zero(dst[i])) {
+				if (ast_strlen_zero(previous_header)) {
 					skipping_headers = 0;
 				}
-				dst[i] = c + 1; /* record start of next line */
+				dst[i] = current_header_offset; /* record start of next line */
 				continue;
 			}
 			if (sipdebug) {
 				ast_debug(4, "%7s %2d [%3d]: %s\n",
 					  req->headers < 0 ? "Header" : "Body",
-					  i, (int) strlen(dst[i]), dst[i]);
+					  i, (int) strlen(previous_header), previous_header);
 			}
-			if (ast_strlen_zero(dst[i]) && req->headers < 0) {
+			if (ast_strlen_zero(previous_header) && req->headers < 0) {
 				req->headers = i;	/* record number of header lines */
 				dst = req->line;	/* start working on the body */
 				i = 0;
@@ -7121,20 +7138,21 @@
 					}
 				}
 			}
-			dst[i] = c + 1; /* record start of next line */
-		}
-        }
+			dst[i] = current_header_offset; /* record start of next line */
+		}
+	}
 
 	/* Check for last header or body line without CRLF. The RFC for SDP requires CRLF,
 	   but since some devices send without, we'll be generous in what we accept. However,
 	   if we've already reached the maximum number of lines for portion of the message
 	   we were parsing, we can't accept any more, so just ignore it.
 	*/
-	if ((i < lim) && !ast_strlen_zero(dst[i])) {
+	previous_header = req->data->str + dst[i];
+	if ((i < lim) && !ast_strlen_zero(previous_header)) {
 		if (sipdebug) {
 			ast_debug(4, "%7s %2d [%3d]: %s\n",
 				  req->headers < 0 ? "Header" : "Body",
-				  i, (int) strlen(dst[i]), dst[i]);
+				  i, (int) strlen(previous_header), previous_header );
 		}
 		i++;
 	}
@@ -7145,7 +7163,8 @@
 	} else {			/* no body */
 		req->headers = i;
 		req->lines = 0;
-		req->line[0] = "";
+		/* req->data->used will be a NULL byte */
+		req->line[0] = ast_str_strlen(req->data);
 	}
 
 	if (*c) {
@@ -7230,25 +7249,26 @@
 	/* search for the boundary marker, the empty line delimiting headers from
 	   sdp part and the end boundry if it exists */
 
-	for (x = 0; x < (req->lines ); x++) {
-		if(!strncasecmp(req->line[x], boundary, strlen(boundary))){
-			if(found_application_sdp && found_end_of_headers){
+	for (x = 0; x < (req->lines); x++) {
+		char *line = REQ_OFFSET_TO_STR(req, line[x]);
+		if (!strncasecmp(line, boundary, strlen(boundary))){
+			if (found_application_sdp && found_end_of_headers) {
 				req->sdp_end = x-1;
 				return 1;
 			}
 			found_application_sdp = FALSE;
 		}
-		if(!strcasecmp(req->line[x], "Content-Type: application/sdp"))
+		if (!strcasecmp(line, "Content-Type: application/sdp"))
 			found_application_sdp = TRUE;
 		
-		if (ast_strlen_zero(req->line[x])) {
-			if(found_application_sdp && !found_end_of_headers){
+		if (ast_strlen_zero(line)) {
+			if (found_application_sdp && !found_end_of_headers){
 				req->sdp_start = x;
 				found_end_of_headers = TRUE;
 			}
 		}
 	}
-	if(found_application_sdp && found_end_of_headers) {
+	if (found_application_sdp && found_end_of_headers) {
 		req->sdp_end = x;
 		return TRUE;
 	}
@@ -8059,9 +8079,9 @@
 	}
 
 	ast_str_append(&req->data, 0, "%s: %s\r\n", var, value);
-	req->header[req->headers] = req->data->str + req->len;
-
-	req->len += strlen(req->header[req->headers]);
+	req->header[req->headers] = req->len;
+
+	req->len = ast_str_strlen(req->data);
 	req->headers++;
 
 	return 0;	
@@ -8086,9 +8106,9 @@
 	if (!req->lines)
 		/* Add extra empty return */
 		req->len += ast_str_append(&req->data, 0, "\r\n");
-	req->line[req->lines] = req->data->str + req->len;
+	req->line[req->lines] = req->len;
 	ast_str_append(&req->data, 0, "%s", line);
-	req->len += strlen(req->line[req->lines]);
+	req->len = ast_str_strlen(req->data);
 	req->lines++;
 	return 0;	
 }
@@ -8304,9 +8324,9 @@
 	resp->method = SIP_RESPONSE;
 	if (!(resp->data = ast_str_create(SIP_MIN_PACKET)))
 		return -1;
-	resp->header[0] = resp->data->str;
+	resp->header[0] = 0;
 	ast_str_set(&resp->data, 0, "SIP/2.0 %s\r\n", msg);
-	resp->len = strlen(resp->header[0]);
+	resp->len = resp->data->used;
 	resp->headers++;
 	return 0;
 }
@@ -8319,9 +8339,9 @@
 	if (!(req->data = ast_str_create(SIP_MIN_PACKET)))
 		return -1;
 	req->method = sipmethod;
-	req->header[0] = req->data->str;
+	req->header[0] = 0;
 	ast_str_set(&req->data, 0, "%s %s SIP/2.0\r\n", sip_methods[sipmethod].text, recip);
-	req->len = strlen(req->header[0]);
+	req->len = ast_str_strlen(req->data);
 	req->headers++;
 	return 0;
 }
@@ -8428,14 +8448,14 @@
 	}
 	
 	if (sipmethod == SIP_CANCEL)
-		c = p->initreq.rlPart2;	/* Use original URI */
+		c = REQ_OFFSET_TO_STR(&p->initreq, rlPart2);	/* Use original URI */
 	else if (sipmethod == SIP_ACK) {
 		/* Use URI from Contact: in 200 OK (if INVITE) 
 		(we only have the contacturi on INVITEs) */
 		if (!ast_strlen_zero(p->okcontacturi))
 			c = is_strict ? p->route->hop : p->okcontacturi;
  		else
- 			c = p->initreq.rlPart2;
+ 			c = REQ_OFFSET_TO_STR(&p->initreq, rlPart2);
 	} else if (!ast_strlen_zero(p->okcontacturi)) 
 		c = is_strict ? p->route->hop : p->okcontacturi; /* Use for BYE or REINVITE */
 	else if (!ast_strlen_zero(p->uri)) 
@@ -9364,8 +9384,6 @@
 /*! \brief copy SIP request (mostly used to save request for responses) */
 static void copy_request(struct sip_request *dst, const struct sip_request *src)
 {
-	long offset;
-	int x;
 	struct ast_str *duplicate = dst->data;
 
 	/* First copy stuff */
@@ -9385,17 +9403,6 @@
 		
 	memcpy(dst->data->str, src->data->str, src->data->used + 1);
 	dst->data->used = src->data->used;
-	offset = ((void *)dst->data->str) - ((void *)src->data->str);
-	/* Now fix pointer arithmetic */
-	for (x = 0; x < src->headers; x++)
-		dst->header[x] += offset;
-	for (x = 0; x < src->lines; x++)
-		dst->line[x] += offset;
-	/* On some occasions this function is called without parse_request being called first so lets not create an invalid pointer */
-	if (src->rlPart1)
-		dst->rlPart1 += offset;
-	if (src->rlPart2)
-		dst->rlPart2 += offset;
 }
 
 /*! \brief Used for 200 OK and 183 early media 
@@ -9427,11 +9434,13 @@
 /*! \brief Parse first line of incoming SIP request */
 static int determine_firstline_parts(struct sip_request *req) 
 {
-	char *e = ast_skip_blanks(req->header[0]);	/* there shouldn't be any */
+	char *e = ast_skip_blanks(req->data->str);	/* there shouldn't be any */
+	char *local_rlPart1;
 
 	if (!*e)
 		return -1;
-	req->rlPart1 = e;	/* method or protocol */
+	req->rlPart1 = e - req->data->str;	/* method or protocol */
+	local_rlPart1 = e;
 	e = ast_skip_nonblanks(e);
 	if (*e)
 		*e++ = '\0';
@@ -9441,10 +9450,10 @@
 		return -1;
 	ast_trim_blanks(e);
 
-	if (!strcasecmp(req->rlPart1, "SIP/2.0") ) { /* We have a response */
+	if (!strcasecmp(local_rlPart1, "SIP/2.0") ) { /* We have a response */
 		if (strlen(e) < 3)	/* status code is 3 digits */
 			return -1;
-		req->rlPart2 = e;
+		req->rlPart2 = e - req->data->str;
 	} else { /* We have a request */
 		if ( *e == '<' ) { /* XXX the spec says it must not be in <> ! */
 			ast_debug(3, "Oops. Bogus uri in <> %s\n", e);
@@ -9452,7 +9461,7 @@
 			if (!*e)
 				return -1; 
 		}
-		req->rlPart2 = e;	/* URI */
+		req->rlPart2 = e - req->data->str;	/* URI */
 		e = ast_skip_nonblanks(e);
 		if (*e)
 			*e++ = '\0';
@@ -12064,7 +12073,7 @@
 
 	/* Find the request URI */
 	if (req->rlPart2)
-		ast_copy_string(tmp, req->rlPart2, sizeof(tmp));
+		ast_copy_string(tmp, REQ_OFFSET_TO_STR(req, rlPart2), sizeof(tmp));
 	
 	if (sip_cfg.pedanticsipchecking)
 		ast_uri_decode(tmp);
@@ -13001,12 +13010,14 @@
 	int y;
 
 	buf[0] = '\0';
+	/*XXX isn't strlen(buf) going to always be 0? */
 	y = len - strlen(buf) - 5;
 	if (y < 0)
 		y = 0;
-	for (x=0; x < req->lines; x++) {
-		strncat(buf, req->line[x], y); /* safe */
-		y -= strlen(req->line[x]) + 1;
+	for (x = 0; x < req->lines; x++) {
+		char *line = REQ_OFFSET_TO_STR(req, line[x]);
+		strncat(buf, line, y); /* safe */
+		y -= strlen(line) + 1;
 		if (y < 0)
 			y = 0;
 		if (y != 0 && addnewline)
@@ -18461,10 +18472,12 @@
 		/* If pedantic is on, we need to check the tags. If they're different, this is
 	   	in fact a forked call through a SIP proxy somewhere. */
 		int different;
+		char *initial_rlPart2 = REQ_OFFSET_TO_STR(&p->initreq, rlPart2);
+		char *this_rlPart2 = REQ_OFFSET_TO_STR(req, rlPart2);
 		if (sip_cfg.pedanticsipchecking)
-			different = sip_uri_cmp(p->initreq.rlPart2, req->rlPart2);
+			different = sip_uri_cmp(initial_rlPart2, this_rlPart2);
 		else
-			different = strcmp(p->initreq.rlPart2, req->rlPart2);
+			different = strcmp(initial_rlPart2, this_rlPart2);
 		if (!different) {
 			transmit_response(p, "482 Loop Detected", req);
 			p->invitestate = INV_COMPLETED;
@@ -18478,12 +18491,12 @@
  			 * \todo XXX This needs to be reviewed.  YOu don't change the request URI really, you route the packet
 			 * correctly instead...
 			 */
-			char *uri = ast_strdupa(req->rlPart2);
+			char *uri = ast_strdupa(this_rlPart2);
 			char *at = strchr(uri, '@');
 			char *peerorhost;
 			struct sip_pkt *pkt = NULL;
 			if (option_debug > 2) {
-				ast_log(LOG_DEBUG, "Potential spiral detected. Original RURI was %s, new RURI is %s\n", p->initreq.rlPart2, req->rlPart2);
+				ast_log(LOG_DEBUG, "Potential spiral detected. Original RURI was %s, new RURI is %s\n", initial_rlPart2, this_rlPart2);
 			}
 			if (at) {
 				*at = '\0';
@@ -20255,7 +20268,7 @@
 
 	/* Get Method and Cseq */
 	cseq = get_header(req, "Cseq");
-	cmd = req->header[0];
+	cmd = REQ_OFFSET_TO_STR(req, header[0]);
 
 	/* Must have Cseq */
 	if (ast_strlen_zero(cmd) || ast_strlen_zero(cseq)) {
@@ -20274,8 +20287,8 @@
 	}
 	/* Get the command XXX */
 
-	cmd = req->rlPart1;
-	e = req->rlPart2;
+	cmd = REQ_OFFSET_TO_STR(req, rlPart1);
+	e = REQ_OFFSET_TO_STR(req, rlPart2);
 
 	/* Save useragent of the client */
 	useragent = get_header(req, "User-Agent");
@@ -20622,7 +20635,7 @@
 		ast_str_reset(req->data); /* nulling this out is NOT a good idea here. */
 		return 1;
 	}
-	req->method = find_sip_method(req->rlPart1);
+	req->method = find_sip_method(REQ_OFFSET_TO_STR(req, rlPart1));
 
 	if (req->debug)
 		ast_verbose("--- (%d headers %d lines)%s ---\n", req->headers, req->lines, (req->headers + req->lines == 0) ? " Nat keepalive" : "");
@@ -20662,7 +20675,7 @@
 	p->recv = *sin;
 
 	if (p->do_history) /* This is a request or response, note what it was for */
-		append_history(p, "Rx", "%s / %s / %s", req->data->str, get_header(req, "CSeq"), req->rlPart2);
+		append_history(p, "Rx", "%s / %s / %s", req->data->str, get_header(req, "CSeq"), REQ_OFFSET_TO_STR(req, rlPart2));
 
 	if (!lockretry) {
 		if (!queue_request(p, req)) {




More information about the asterisk-commits mailing list