Summary:	ASTERISK-05714: Embedded SIP reserved characters may cause indigestion to cdr-csv
Reporter:	John Todd (jtodd)	Labels:
Date Opened:	2005-11-25 20:26:42.000-0600	Date Closed:	2005-12-03 14:25:29.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	CDR/cdr_csv
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	I'm not quite sure if this is a security flaw or simply a "tripwire" for the unwary, but I think it can be easily solved. Consider this CDR: "","jtodd","echo","from-internet","""John,Todd"" <jtodd>","SIP/loligo.com-08193608","","Hangup","","2005-11-26 02:17:07","2005-11-26 02:17:07","2005-11-26 02:17:09",2,2,"ANSWERED","DOCUMENTATION"   Note the extra comma in my name.  I set that in Eyebeam, totally externally of Asterisk.  I'm sure that a reasonably clever person could put all sorts of nasty things in there which would trip up most CDR parsing engines that use commas in this file as delimiters - I'll leave the nasty bits to the imagination of the reader. Perhaps we should not entrust remote systems to encode reserved characters into their hex code point equivalents.  I am a mere mortal, and cannot code this patch myself, but I would suggest that perhaps someone might consider parsing From: header contents and escaping the dangerous bits (commas, other?) into %2C (hex-style) format, at least before insertion into the cdr-csv file format.
Comments:	By: Olle Johansson (oej) 2005-11-29 01:20:07.000-0600 This will be an even bigger issue when we move into UTF8 format for all IAX2 and SIP usernames and caller IDs. What format are we going to log in? What format are we going to store CDR's in? Short term, we could decide to URIencode these names, but that will be a major change. By: John Todd (jtodd) 2005-11-29 22:01:13.000-0600 I agree that it has to be considered in the broad case of conversion to UTF8.  I don't think I'd suggest URIencoding the whole name at this point before that work is done, but instead focus on a more pragmatic removal or selective URLencoding of "," characters from entries before they find their way to cdr-csv. I see an immediate security (but mostly "billing") risk with "," embedded into portions of the CDR in an un-escaped manner.  Other characters may also pose a risk, but it seems trivial to cause havoc if one's system does not "force" the username portions of the SIP messages via configurations in sip.conf and instead trust what is presented from the UA. By: Tilghman Lesher (tilghman) 2005-12-02 11:30:22.000-0600 jtodd:  only simplistic parsers are likely to be tripped up with an embedded comma.  Most CSV parsers already deal with the possibility of embedded commas by requiring that the arguments be quoted, which is exactly what is done here. Or in other words, we can't protect against incompetent coders, but if we follow the standard, that clearly puts the onus on the parser to work correctly. By: John Todd (jtodd) 2005-12-02 12:48:42.000-0600 OK, I can agree with that.  The risk still remains, but we're shifting it to the CDR processor which may or may not be reasonable. Having a method to remove (not merely quote-contain) dangerous values in SIP CDR fields is still also a reasonable thing to do, but not "necessary". This is a flaw that may exist across many SIP platforms and billing systems; I was hoping that perhaps when someone else starts this up as a security discussion that it would be possible to say "The Asterisk community recognized this X months ago, and there exists the blah-foo-meow option in the CDR subsystem to protect less-than-optimal billing parsers from fraud attempts, if you so choose." Anyway, the quoting is probably sufficient at the least.  Resolving/closing. By: Olle Johansson (oej) 2005-12-02 13:40:20.000-0600 Conclusion: We need some filter hooks in the CDR storage as we move forward to make sure that entries comply with local policy for the specific storage. We also might want to have some filters to filter out caller ID names and addresses that may be risky or simply not comply with local policy. By: Tilghman Lesher (tilghman) 2005-12-03 14:25:28.000-0600 I'm going to go ahead and close this out; we can revisit filters at a later date, when we have a proposed patch.