[asterisk-bugs] [Asterisk 0012036]: [patch] RFC 3372 SIP-T receive implementation

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=12036 
====================================================================== 
Reported By:                gasparz
Assigned To:                file
====================================================================== 
Project:                    Asterisk
Issue ID:                   12036
Category:                   Channels/NewFeature
Reproducibility:            always
Severity:                   feature
Priority:                   normal
Status:                     assigned
Asterisk Version:           1.4.17 
Regression:                 No 
SVN Branch (only for SVN checkouts, not tarball releases): N/A 
SVN Revision (number only!):  
Request Review:              
====================================================================== 
Date Submitted:             2008-02-20 07:47 CST
Last Modified:              2009-04-13 19:06 CDT
====================================================================== 
Summary:                    [patch] RFC 3372 SIP-T receive implementation
Description: 
The patch enables you to receive sip-t packets and decodes some fields.

The sip-t means that in the initial invite the content is multipart with
1) the sdp part
2) application/isup part
this second part contains the IAM message from the SS7. This is cool
because it contains some fields that are not available in the basic sip
packet like:
Origination line:
0- Plain Old Telephone Service (POTS) - non-coin service requiring no
special treatment
70- Code 70 identifies a line connected to a pay station (including both
coin and coinless stations) which does not use network provided coin
control signaling.  II 70 is used to identify this type pay station line
irrespective of whether the pay station is provided by a LEC or a non-LEC. 
II 70 is transmitted from the originating end office on all calls made from
these lines.

For some carriers this is the method to send the payphone information. If
you use toll free numbers this is a cool feature to have, so that you can
bill the payphone too.

Let me know how it worked for you. (For me it's working well).

Cheers
======================================================================
Relationships       ID      Summary
----------------------------------------------------------------------
related to          0013495 [patch] isup timers + q.764 compatibili...
====================================================================== 

---------------------------------------------------------------------- 
 (0103191) dimas (reporter) - 2009-04-13 19:06
 http://bugs.digium.com/view.php?id=12036#c103191 
---------------------------------------------------------------------- 
I have nothing to say about the feature itself (I'm not using it) but would
like to put couple of comment about coding style:

1.
+	char content_type_isup[35] = "Content-Type: application/isup\0";

AFAIK there is no need to specify the size and it is more prone to
errors.

2. there are lots of loops where condition looks like xxx < length(yyy).
Nothing serious, just see no need making O(n) algorithm O^2(n)...

3. the 

  while(req->data->str[k+i+9] != '\r')

loop will just produce crash with improperly (intentionally or not)
formatted message. Also, nobody checks that multipart_message_boundary
buffer is large enough. The straight way to remote exploitable DoS...
I personally would just do strchr followed by the NULL check and strncpy.

4. Formatting issues:
* no spaces after 'if' keyword, no spaces before 'while' in do/while...
* there are binary operators (+) not surrounded by spaces. 
* other issues probably caused by mix of tabs and spaces.

5. The code is just full of constructs like:

   isup.hop_counter[k] = 48 + (int)((unsigned
char)(req->data->str[isup_offset+1])) / 10;

If you need to cast to unsigned char that many times, why not to declare 
unsigned char *str = (unsigned char *) req->data->str;
and use it? which will give

   isup.hop_counter[k] = 48 + str[isup_offset+1] / 10;

And also 48 may be better expressed as '0' - there are enough other magic
constants in the code :) 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2009-04-13 19:06 dimas          Note Added: 0103191                          
======================================================================