Hello,
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed. According to the BNF of 3261 generic parameters without parameters are allowed in To and From header.
So I created the attached patch, which seems to work for me. Please review carefully and let me know if I overlooked something in the problem statement or the bug fix.
Cheers Nils
Index: parse_to.c =================================================================== RCS file: /cvsroot/ser/sip_router/parser/parse_to.c,v retrieving revision 1.22 diff -a -u -r1.22 parse_to.c --- parse_to.c 23 Jun 2008 17:09:57 -0000 1.22 +++ parse_to.c 20 May 2009 05:04:40 -0000 @@ -122,6 +122,11 @@ case TAG3: param->type=TAG_PARAM; case PARA_NAME: + param->value.len = tmp-param->value.s; + saved_status = E_PARA_VALUE; + status = F_LF; + add_param( param , to_b ); + break; case TAG1: case TAG2: param->name.len = tmp-param->name.s; @@ -161,6 +166,11 @@ case TAG3: param->type=TAG_PARAM; case PARA_NAME: + param->name.len = tmp-param->name.s; + saved_status = E_PARA_VALUE; + status = F_CR; + add_param( param , to_b ); + break; case TAG1: case TAG2: param->name.len = tmp-param->name.s; @@ -277,7 +287,13 @@ } #endif case PARA_VALUE_TOKEN: - param->value.len=tmp-param->value.s; + case PARA_NAME: + if (status == PARA_VALUE_TOKEN) { + param->value.len=tmp-param->value.s; + } + else { + param->name.len = tmp-param->name.s; + } add_param(param,to_b); case E_PARA_VALUE: param = (struct to_param*)
Hi,
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed. According to the BNF of 3261 generic parameters without parameters are allowed in To and From header.
as I received so far no feedback at all for this issue I'm wondering if everybody is just fine with the fact that sip-router can not parse parameter only values in To and From header (and maybe other headers which use the parse_to function). Or is everybody just still puzzled by this fact and still reviewing my patch?
Regards Nils
Nils Ohlmeier writes:
as I received so far no feedback at all for this issue I'm wondering if everybody is just fine with the fact that sip-router can not parse parameter only values in To and From header (and maybe other headers which use the parse_to function).
in my opinion, if a patch is submitted, it should be reviewed and reacted to. i personally have not yet seen to/from params without values.
-- juha
El Jueves, 28 de Mayo de 2009, Juha Heinanen escribió:
i personally have not yet seen to/from params without values.
But they are perfectly valid :(
Am 28.05.2009 21:17 Uhr, schrieb Juha Heinanen:
Iñaki Baz Castillo writes:
i personally have not yet seen to/from params without values.
But they are perfectly valid :(
sure. i just wanted to say that i have not yet been personally hit by this parsing problem.
Right. If someone would have stumbeled about this, I guess we should have received reports/complaints earlier. So it is more a theoretical problem then a real one, which results in lower priority. But still it seems that we agree that it should be fixed.
Nils
On May 28, 2009 at 17:44, Nils Ohlmeier nils@iptel.org wrote:
Hi,
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed. According to the BNF of 3261 generic parameters without parameters are allowed in To and From header.
as I received so far no feedback at all for this issue I'm wondering if everybody is just fine with the fact that sip-router can not parse parameter only values in To and From header (and maybe other headers which use the parse_to function). Or is everybody just still puzzled by this fact and still reviewing my patch?
There were some problems with your patch. You assumed that CR or LF will mark the end of the parameter, but you can have the parameter continued on the next line. There was also a mixed value with name. I have a different patch, which I'll commit into andrei/to_parser_fix branch. It removes the special PINGTEL cases and fixes another theoretical problem when a parameter is at the end of string (not terminated by CR or LF, but by the end of the string/message).
However I did only _very_ little testing. Could you run your tests on it? I don't want to merge the to_fix unless we are reasonably sure that it's good and it does not introduce some other bug.
Andrei
Am 28.05.2009 20:46 Uhr, schrieb Andrei Pelinescu-Onciul:
On May 28, 2009 at 17:44, Nils Ohlmeiernils@iptel.org wrote:
Hi,
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To:sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed. According to the BNF of 3261 generic parameters without parameters are allowed in To and From header.
as I received so far no feedback at all for this issue I'm wondering if everybody is just fine with the fact that sip-router can not parse parameter only values in To and From header (and maybe other headers which use the parse_to function). Or is everybody just still puzzled by this fact and still reviewing my patch?
There were some problems with your patch. You assumed that CR or LF will mark the end of the parameter, but you can have the parameter continued on the next line. There was also a mixed value with name. I have a different patch, which I'll commit into andrei/to_parser_fix branch. It removes the special PINGTEL cases and fixes another theoretical problem when a parameter is at the end of string (not terminated by CR or LF, but by the end of the string/message).
However I did only _very_ little testing. Could you run your tests on it? I don't want to merge the to_fix unless we are reasonably sure that it's good and it does not introduce some other bug.
Ok I will give detailed testing. But that could take some time.
BTW the To parser has even with this patch some more problems, e.g. null characters in display names are not treated correctly. So maybe we could take this branch to improve the To parser, give it real hard testing and then merge it later.
Nils
2009/5/28 Nils Ohlmeier nils@iptel.org:
Hi,
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed.
Hi Nils, I'm trying it in Kamailio 1.5 (sending a request with "To: sip:nils@invalid.com;foo") and no error occurs. Does it just affect to SR?
Hi,
2009/5/28 Nils Ohlmeier nils@iptel.org:
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed.
Hi Nils, I'm trying it in Kamailio 1.5 (sending a request with "To: sip:nils@invalid.com;foo") and no error occurs. Does it just affect to SR?
I would think that Kamailio is affected as well, because To header parameter parser code is in the SER CVS since 2002 and was basically not touched since then. So if not somebody in the Kamilio project fixed this it should be the same. During my tests I found out that it is important that you are using a configuration which actually tries to parse the To header at all. For example if you send an OPTIONS request to the iptel.org server (not a user), the To header can be as ugly as you want (you will always get a 200 OK), because the configuration does not care the To header at all. But if you send a REGISTER with such a To header you will not get any response at all.
Regards Nils
El Viernes, 29 de Mayo de 2009, Nils Ohlmeier escribió:
Hi,
2009/5/28 Nils Ohlmeier nils@iptel.org:
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed.
Hi Nils, I'm trying it in Kamailio 1.5 (sending a request with "To: sip:nils@invalid.com;foo") and no error occurs. Does it just affect to SR?
I would think that Kamailio is affected as well, because To header parameter parser code is in the SER CVS since 2002 and was basically not touched since then. So if not somebody in the Kamilio project fixed this it should be the same. During my tests I found out that it is important that you are using a configuration which actually tries to parse the To header at all. For example if you send an OPTIONS request to the iptel.org server (not a user), the To header can be as ugly as you want (you will always get a 200 OK), because the configuration does not care the To header at all. But if you send a REGISTER with such a To header you will not get any response at all.
Nils, I'm sending REGISTER with "To: sip:anonimous@domain.org;qwe" and Kamailio gives no error (and stores the user location right.
El Viernes, 29 de Mayo de 2009, Nils Ohlmeier escribió:
2009/5/28 Nils Ohlmeier nils@iptel.org:
during testing SER at SIPit 24 I discovered something what I believe
is
a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed.
Hi Nils, I'm trying it in Kamailio 1.5 (sending a request with "To: sip:nils@invalid.com;foo") and no error occurs. Does it just affect to SR?
I would think that Kamailio is affected as well, because To header parameter parser code is in the SER CVS since 2002 and was basically not touched since then. So if not somebody in the Kamilio project fixed this it should be the same. During my tests I found out that it is important that you are using a configuration which actually tries to parse the To header at all. For example if you send an OPTIONS request to the iptel.org server (not a user), the To header can be as ugly as you want (you will always get a 200 OK), because the configuration does not care the To header at all. But if you send a REGISTER with such a To header you will not get any response at all.
Nils, I'm sending REGISTER with "To: sip:anonimous@domain.org;qwe" and Kamailio gives no error (and stores the user location right.
Hmmm that is strange, because from looking at the Kamailio code via viewsvn it looks identical to the one from SER. I'll try to reproduce it with Kamailio myself, but that could take some time.
Regards Nils
-
Andrei Pelinescu-Onciul -
Iñaki Baz Castillo -
jh＠tutpro.com -
Nils Ohlmeier -
Nils Ohlmeier