looks like sr parser/parse_event.c has the same bug that i found in
opensips although the implementations are different.
after statement
end = skip_token(tmp.s, tmp.len);
end points to next char after event string.
then end is assigned to tmp.s
tmp.s = end;
and after that if statement compares this next char to ';':
if (tmp.s[0] == ';') {
which looks wrong to me, because that char can be anything if event does
not have any params.
perhaps someone else can recheck if i'm right or wrong.
-- juha
THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY.
A new Flyspray task has been opened. Details are below.
User who did this - Anonymous user ()
Attached to Project - sip-router
Summary - ycckgrbg
Task Type - Improvement
Category - tcp
Status - Assigned
Assigned To - Andrei Pelinescu-Onciul
Operating System - All
Severity - Critical
Priority - Normal
Reported Version - Development
Due in Version - Undecided
Due Date - Undecided
Details - izrnsdtc http://cvgksuzd.com jsqbvodl nnefuqxi [URL=http://styynjnv.com]rhsnsyzz[/URL] <a href="http://maivmndq.com">gvcpvnuc</a>
More information can be found at the following URL:
http://sip-router.org/tracker/index.php?do=details&task_id=19
You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.
THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY.
A user has added themself to the list of users assigned to this task.
FS#19 - ycckgrbg
User who did this - Anonymous user ()
http://sip-router.org/tracker/index.php?do=details&task_id=19
You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.
Module: sip-router
Branch: sr_3.0
Commit: 9cb346dec84f191a53d1dbe5895f2e4276da67fe
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=9cb346d…
Author: Jan Janak <jan(a)ryngle.com>
Committer: Jan Janak <jan(a)ryngle.com>
Date: Sun Oct 18 20:49:39 2009 +0200
event parser: Add missing string boundary checks to event_parser func.
The function event_parser needs to check that there is still some text
left in the input string before it attempts to read the text.
In addition to that the function also needs to skip any possible
leading whitespace before it calls parse_params, because parse_params
expects that there is no leading whitespace at the beginning of the
input string.
Reported by Juha Heinanen
---
parser/parse_event.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/parser/parse_event.c b/parser/parse_event.c
index 3d4b6b2..8adac43 100644
--- a/parser/parse_event.c
+++ b/parser/parse_event.c
@@ -117,10 +117,16 @@ int event_parser(char* s, int len, event_t* e)
tmp.s = end;
trim_leading(&tmp);
- if (tmp.s[0] == ';') {
+ e->params.list = NULL;
+
+ if (tmp.len && (tmp.s[0] == ';')) {
+ /* Shift the semicolon and skip any leading whitespace, this is needed
+ * for parse_params to work correctly. */
+ tmp.s++; tmp.len--;
+ trim_leading(&tmp);
+ if (!tmp.len) return 0;
+
/* We have parameters to parse */
- tmp.s++;
- tmp.len--;
if (e->type == EVENT_DIALOG) {
pclass = CLASS_EVENT_DIALOG;
phooks = (param_hooks_t*)&e->params.dialog;
@@ -130,10 +136,7 @@ int event_parser(char* s, int len, event_t* e)
ERR("event_parser: Error while parsing parameters parameters\n");
return -1;
}
- } else {
- e->params.list = NULL;
}
-
return 0;
}
Module: sip-router
Branch: master
Commit: 0f828dfbcab468b4fe7f8aec640587acf3e381c3
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=0f828df…
Author: Jan Janak <jan(a)ryngle.com>
Committer: Jan Janak <jan(a)ryngle.com>
Date: Sun Oct 18 20:49:39 2009 +0200
event parser: Add missing string boundary checks to event_parser func.
The function event_parser needs to check that there is still some text
left in the input string before it attempts to read the text.
In addition to that the function also needs to skip any possible
leading whitespace before it calls parse_params, because parse_params
expects that there is no leading whitespace at the beginning of the
input string.
Reported by Juha Heinanen
---
parser/parse_event.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/parser/parse_event.c b/parser/parse_event.c
index 3d4b6b2..8adac43 100644
--- a/parser/parse_event.c
+++ b/parser/parse_event.c
@@ -117,10 +117,16 @@ int event_parser(char* s, int len, event_t* e)
tmp.s = end;
trim_leading(&tmp);
- if (tmp.s[0] == ';') {
+ e->params.list = NULL;
+
+ if (tmp.len && (tmp.s[0] == ';')) {
+ /* Shift the semicolon and skip any leading whitespace, this is needed
+ * for parse_params to work correctly. */
+ tmp.s++; tmp.len--;
+ trim_leading(&tmp);
+ if (!tmp.len) return 0;
+
/* We have parameters to parse */
- tmp.s++;
- tmp.len--;
if (e->type == EVENT_DIALOG) {
pclass = CLASS_EVENT_DIALOG;
phooks = (param_hooks_t*)&e->params.dialog;
@@ -130,10 +136,7 @@ int event_parser(char* s, int len, event_t* e)
ERR("event_parser: Error while parsing parameters parameters\n");
return -1;
}
- } else {
- e->params.list = NULL;
}
-
return 0;
}
i tested with an older version of sr and found that $du setting in
branch_route has not effect also there. so looks like this has nothing
to do with jan's t_serial commit.
anyway, this is a serious issue and should get fixed.
-- juha
i started testing serial forking, but before i got that far, i noticed
that $du that i set in branch route, has stopped working.
in branch_route i have:
$du = "sip:127.0.0.1:5070";
xlog("L_INFO", "du is <$du>\n");
and i get to syslog
Oct 18 14:54:54 localhost /usr/sbin/sip-proxy[18707]: INFO: du is <sip:127.0.0.1:5070>
but wireshark shows that the request is not routed to $du, but according
to r-uri.
i'm pretty sure that this worked earlier.
-- juha
Module: sip-router
Branch: sr_3.0
Commit: 2ea8963512096cbb54e6fdf54e7b44eacc371f34
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=2ea8963…
Author: Jan Janak <jan(a)ryngle.com>
Committer: Jan Janak <jan(a)ryngle.com>
Date: Sun Oct 18 00:39:31 2009 +0200
tm: Number of fixes in code and documentation for serial forking.
This patch improves the serial forking related code and documentation.
The original code suffers from shortcommits, some of which were result
of the migration to the new core.
The function that decodes destination sets encoded in AVPs has been
improved, because the original version did not handle properly strings
with missing elements. In such case the original implementation was
likely to overwrite memory, because it did not check the return value
of strchr properly. The new implementation tries to handle this
situation. It continues parsing as long as it can, it only requires
that the request-uri string is present, all other fields are made
optional and their variables are properly initialized if their values
cannot be found in the AVP.
There was a bug in the implementation of fr_inv_timer_next modparam in
the original version, changes to the parameter value were ignored by
the serial forking code. This was reported by Andrei and is now fixed
by this commit. The parameter fr_inv_timer_next can now be configured
at runtime with the configuration framework. Its value is in
milliseconds and unlike fr_inv_timer, this timer cannot be configured
separately for individual branches.
Obsolete definition of INV_FR_TIME_OUT_FIRST has been removed. That
macro is not used anywhere in the code, thus it is not needed.
There were several places where LM_DBG printed a string and relied on
the string being zero terminated, this may or may not be true in the
future and this patch uses the macro STR_FMT where appropriate, which
is safer.
Function t_next_contacts now checks if the function decode_branch_info
really returned values for the dst_uri and the path vector. If not then
it calls reset_dst_uri and reset_path_vector. Previous version of the
code crashed sip-router, this is likely due to the merge and updates in
the sip-router core.
We now use t_set_fr in t_next_contacts for setting the fr_inv_timer
value to fr_inv_timer_next. This is much more efficient than creating
AVPs with new timer values. Also the new value of the timer is now
taken from a variable in the configuration framework, instead of just a
regular global variable configured through modparam. This way we can
adjust the value of the timer on the fly. Configuring it through
modparam is, of course, possible too.
The value of of fr_inv_timer_next is now in milliseconds, instead of
seconds. That's the only possibly incompatible change. However, this is
consistent with all other timers in tm module, it is more efficient and
it offers better granularity.
A missing call to destroy_avp has been added to t_next_contacts, in the
code which is executed when no transaction exists. There, the avp
should also be destroyed if all values have been exhausted and none of
them had Q_FLAG set. This is a corner case which should not happen
under normal circumstances, because that situation only happens if all
branches have the same q value. Such AVP would not have been created by
t_load_contacts and therefore t_next_contacts should not be called, but
this bug may be triggered if someone uses t_next_contacts in an
unexpected way and it is probably better to have it fixed.
Also the code which restores the value of fr_inv_timer at the end of
t_next_contacts did not work properly. This patch fixes that. It first
tries to retrieve a value configured with t_set_fr, but that is not
guaranteed to succeed. After that it also tries the timer AVP and
finally the configuration framework. The configuration framework always
yields a value, so we can always restore the timer value, but we may
fail to restore individual transaction timer values set by t_set_fr. If
that fails then the global value from the configuration framework is
used. This is documented as a shortcomming in the README and in the
code.
In addition to code changes this patch also expands documentation on
functions t_load_contacts and t_next_contacts, describing their
operation in more detail. Also the format of the contacts AVP is now
documented.
Finally, there is a whole new section in the README which describes
how serial/parallel forking can be achieved with t_load_contacts and
t_next_contacts and provides a number of examples.
---
modules/tm/README | 592 +++++++++++++++++++++++++++---------------
modules/tm/config.c | 7 +-
modules/tm/config.h | 3 +-
modules/tm/doc/functions.xml | 129 ++++++++--
modules/tm/doc/params.xml | 83 +++++--
modules/tm/doc/tm.xml | 218 ++++++++++++++++
modules/tm/t_serial.c | 208 ++++++++++------
modules/tm/tm.c | 2 +-
8 files changed, 900 insertions(+), 342 deletions(-)
Diff: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=2ea…