This issue has been migrated from Redmine: https://dev.icinga.com/issues/1047
Created by jw on 2010-12-08 16:48:42 +00:00
Status: Resolved (closed on 2011-01-19 09:50:25 +00:00)
Target Version: 1.3
Last Update: 2014-12-08 14:35:37 +00:00 (in Redmine)
Icinga Version: 1.10.0
OS Version: any
ido2db uses strtok to split the Command-String into Command and Arguments:
char *cmdptr = NULL;
char *argptr = NULL;
cmdptr = strtok(idi->buffered_input[IDO_DATA_HOSTCHECKCOMMAND], "!");
argptr = strtok(NULL, "\x0");
If there is no argument in the configuration, the second strtok returns NULL wich leads to segmentation faults(at least on solaris) if argptr got dereferenced later:
es = ido2db_db_escape_string(idi, argptr);
The attached patch fixed that problem by setting the es strings to "\0" istead of escaping the NULL-ptr content if argptr is NULL
2011-01-05 16:20:51 +00:00 by mfriedrich 2fadc0c
idoutils: fix ido2db needs to check for empty command arguments on Solaris (Julian Wiesner) #1047
2011-01-13 14:46:44 +00:00 by mfriedrich 22800c3
2011-01-19 09:18:07 +00:00 by mfriedrich e73f020
idoutils: fix check for empty command args in host|service|notification definitions #1047
this is clearly checking the values and putting an empty string instead of NULL
into asprintf, not violating the Solaris convention.
2011-01-20 11:57:57 +00:00 by mfriedrich 6d5ba04
Updated by mfriedrich on 2010-12-17 16:19:01 +00:00
Updated by mfriedrich on 2010-12-17 16:19:16 +00:00
thanks. i'll add that to my todo list.
Updated by mfriedrich on 2011-01-05 16:21:22 +00:00
the main problem is that passing a NULL value to ido2db_db_escape_string will return a NULL value into es array member.
this will passed via adress pointer up into data which then will be processed within the ido2db_query_insert_or_update queries where the asprintf into a query buffer takes place. at least mysql and postgresql are using that (oracle uses binded params where this won't happen).
so the main problem is within there,
whereas those are defined in dbqueries.c. protecting asprintf won't make any sense as it will throw the segfault either way.
so i think your patch attempt is fine. it is applied in my mfriedrich/core branch and will run into testing, then merged into git master for 1.3 release.
thanks for the patch, more welcome :-)
Updated by mfriedrich on 2011-01-05 18:09:50 +00:00
Applied in changeset commit:"2fadc0c66ea7a45f1f451409ab9180567bc673ce".
Updated by mfriedrich on 2011-01-19 08:04:05 +00:00
i need to reopen this as it's not the correct solution.
passing \0 into a string causes this to be a string literal, on which a free() will segfault (at least on linux/unix based systems).
so the correct implementation would be to check if the value is NULL and if yes, add "" instead of NULL.
Updated by mfriedrich on 2011-01-19 09:17:37 +00:00
Updated by mfriedrich on 2011-01-19 09:50:25 +00:00
Applied in changeset commit:"e73f0206e04f235fcabc9501f658c19b3158c27f".
Updated by mfriedrich on 2014-12-08 14:35:37 +00:00