[reSIProcate] Hardening the parser (what goes into 1.0?)

[Byron Campen]

	A summary of the issues that I have found so far in the parse code  
while working with the torture tests:

1. ParserCategory will accept empty and repeated parameters.
2. In general, overflow checking is not performed on header-field- 
values/parameters that take integral values. The parse code is  
yielding garbage when these are not within the range of an int (in  
most cases, this is not enough to handle what the RFCs say are valid).
3. Content-Length is not processed according to RFC 3261 Sec 18.3  
(ie, it is ignored in the UDP case)
4. Multiple values are allowed (and re-emitted) in single-value headers.

On point 1:
	We haven't taken any action yet. David queried whether it had been  
decided that tracking uniqueness of parameters was too expensive. I  
made the point that this could be leveraged in a DoS attack by  
putting very large numbers of empty parameters (ie. semi-colons) in a  
header-field-value, causing the data structures used to hold these  
parameters to grow very large.

On point 2:
	I have some code in a working-copy that has (mostly) replaced  
IntegerCategory and IntegerParameter with the more descriptive (and  
overflow safe) UInt32Category, UInt8Category, UInt32Parameter, and  
UInt8Parameter. All the necessary changes to Headers.?xx,  
HeaderTypes.hxx, SipMessage.?xx, ParameterTypes.?xx,  
ParameterTypeEnums.hxx, and ParserCategory.?xx have been made. Do  
people want to see this in a branch before we work it into the main  
trunk? Also, is this something we want to push into the release?

On point 3:
	Robert has suggested that the stack's treatment of Content-Length  
should be tunable; by default it would function according to sec  
18.3, but it could be configured to behave as it does now. I have not  
implemented the configurable behavior yet, but I do have code that  
implements 18.3, again in a working copy. Does this need to make it  
into a branch first? Is this something we want to put in the release?  
(Note that 2 and 3 are closely-tied here; if we are going to be  
taking Content-Length seriously, we NEED overflow-safe integral value  
parse code, or we will end up dumping core when someone sends a bad  
Content-Length value at us.)

On point 4:
	I have code in a working copy that checks for this problem within  
SipMessage::addHeader(). Once again, does anyone want to review this  
stuff in a branch, and do we think this should make it in 1.0?

Best regards,
Byron Campen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2369 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20060807/7e653d44/attachment.bin>