* Re: [conntrack-tools] XML output is invalid [not found] <F7825499-A8F5-4F41-B3DD-2E13807954E2@rotsen.be> @ 2008-06-20 13:24 ` Pablo Neira Ayuso 2008-06-20 13:30 ` Patrick McHardy 2008-06-20 14:03 ` Jan Engelhardt 0 siblings, 2 replies; 6+ messages in thread From: Pablo Neira Ayuso @ 2008-06-20 13:24 UTC (permalink / raw) To: Sen Haerens; +Cc: Netfilter Development Mailinglist [-- Attachment #1: Type: text/plain, Size: 627 bytes --] Sen Haerens wrote: > Hi Pablo, > > I wanted to enter this into Bugzilla but I can't pick the > conntrack-tools product when entering a bug. I'll check what happen with this. > Outputting XML from conntrack produces an invalid XML document: > > * The XML declaration is missing: <?xml version="1.0"?> > * There can only be one root element. We have to wrap all the flow > elements into a single root element: > <conntrack> > <flow>[...]</flow> > <flow>[...]</flow> > [...] > </conntrack> > > My version: conntrack v0.9.7 (conntrack-tools) Does this patch help? -- "Los honestos son inadaptados sociales" -- Les Luthiers [-- Attachment #2: x --] [-- Type: text/plain, Size: 2761 bytes --] diff --git a/src/conntrack.c b/src/conntrack.c index 25a3a57..6b43352 100644 --- a/src/conntrack.c +++ b/src/conntrack.c @@ -604,10 +604,16 @@ static int ignore_nat(const struct nf_conntrack *obj, } static int counter; +static int dump_xml_header_done = 1; static void __attribute__((noreturn)) event_sighandler(int s) { + if (dump_xml_header_done == 0) { + printf("</conntrack>\n"); + fflush(stdout); + } + fprintf(stderr, "%s v%s: ", PROGNAME, VERSION); fprintf(stderr, "%d flow events has been shown.\n", counter); nfct_close(cth); @@ -619,6 +625,7 @@ static int event_cb(enum nf_conntrack_msg_type type, void *data) { char buf[1024]; + int len = 0; struct nf_conntrack *obj = data; unsigned int op_type = NFCT_O_DEFAULT; unsigned int op_flags = 0; @@ -629,8 +636,14 @@ static int event_cb(enum nf_conntrack_msg_type type, if (options & CT_COMPARISON && !nfct_cmp(obj, ct, NFCT_CMP_ALL)) return NFCT_CB_CONTINUE; - if (output_mask & _O_XML) + if (output_mask & _O_XML) { op_type = NFCT_O_XML; + if (dump_xml_header_done) { + dump_xml_header_done = 0; + len = snprintf(buf, 1024, "<?xml version=\"1.0\"?>\n" + "<conntrack>\n"); + } + } if (output_mask & _O_EXT) op_flags = NFCT_OF_SHOW_LAYER3; if (output_mask & _O_TMS) { @@ -644,7 +657,7 @@ static int event_cb(enum nf_conntrack_msg_type type, if (output_mask & _O_ID) op_flags |= NFCT_OF_ID; - nfct_snprintf(buf, 1024, ct, type, op_type, op_flags); + nfct_snprintf(buf+len, 1024-len, ct, type, op_type, op_flags); printf("%s\n", buf); fflush(stdout); @@ -658,6 +671,7 @@ static int dump_cb(enum nf_conntrack_msg_type type, void *data) { char buf[1024]; + int len = 0; struct nf_conntrack *obj = data; unsigned int op_type = NFCT_O_DEFAULT; unsigned int op_flags = 0; @@ -668,14 +682,20 @@ static int dump_cb(enum nf_conntrack_msg_type type, if (options & CT_COMPARISON && !nfct_cmp(obj, ct, NFCT_CMP_ALL)) return NFCT_CB_CONTINUE; - if (output_mask & _O_XML) + if (output_mask & _O_XML) { op_type = NFCT_O_XML; + if (dump_xml_header_done) { + dump_xml_header_done = 0; + len = snprintf(buf, 1024, "<?xml version=\"1.0\"?>\n" + "<conntrack>\n"); + } + } if (output_mask & _O_EXT) op_flags = NFCT_OF_SHOW_LAYER3; if (output_mask & _O_ID) op_flags |= NFCT_OF_ID; - nfct_snprintf(buf, 1024, ct, NFCT_T_UNKNOWN, op_type, op_flags); + nfct_snprintf(buf+len, 1024-len, ct, NFCT_T_UNKNOWN, op_type, op_flags); printf("%s\n", buf); counter++; @@ -1129,6 +1149,11 @@ int main(int argc, char *argv[]) else res = nfct_query(cth, NFCT_Q_DUMP, &family); + if (dump_xml_header_done == 0) { + printf("</conntrack>\n"); + fflush(stdout); + } + nfct_close(cth); break; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [conntrack-tools] XML output is invalid 2008-06-20 13:24 ` [conntrack-tools] XML output is invalid Pablo Neira Ayuso @ 2008-06-20 13:30 ` Patrick McHardy 2008-06-20 13:52 ` Pablo Neira Ayuso 2008-06-20 14:03 ` Jan Engelhardt 1 sibling, 1 reply; 6+ messages in thread From: Patrick McHardy @ 2008-06-20 13:30 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Sen Haerens, Netfilter Development Mailinglist Pablo Neira Ayuso wrote: > Does this patch help? > > > + if (output_mask & _O_XML) { > op_type = NFCT_O_XML; > + if (dump_xml_header_done) { > + dump_xml_header_done = 0; > + len = snprintf(buf, 1024, "<?xml version=\"1.0\"?>\n" > + "<conntrack>\n"); > + } > + } > if (output_mask & _O_EXT) > op_flags = NFCT_OF_SHOW_LAYER3; > if (output_mask & _O_ID) > op_flags |= NFCT_OF_ID; > > - nfct_snprintf(buf, 1024, ct, NFCT_T_UNKNOWN, op_type, op_flags); > + nfct_snprintf(buf+len, 1024-len, ct, NFCT_T_UNKNOWN, op_type, op_flags); It doesn't seem to matter here, but that looks buggy (combined with the snprintf above). When the buffer size is exceed, snprintf returns the amount of characters it *would have written* if enough space was available. So when this really happens above, you have a buffer overflow in the second snprintf. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [conntrack-tools] XML output is invalid 2008-06-20 13:30 ` Patrick McHardy @ 2008-06-20 13:52 ` Pablo Neira Ayuso 2008-06-20 14:05 ` Patrick McHardy 2008-06-20 14:06 ` Pablo Neira Ayuso 0 siblings, 2 replies; 6+ messages in thread From: Pablo Neira Ayuso @ 2008-06-20 13:52 UTC (permalink / raw) To: Patrick McHardy; +Cc: Sen Haerens, Netfilter Development Mailinglist Patrick McHardy wrote: > Pablo Neira Ayuso wrote: >> Does this patch help? >> >> >> + if (output_mask & _O_XML) { >> op_type = NFCT_O_XML; >> + if (dump_xml_header_done) { >> + dump_xml_header_done = 0; >> + len = snprintf(buf, 1024, "<?xml version=\"1.0\"?>\n" >> + "<conntrack>\n"); >> + } >> + } >> if (output_mask & _O_EXT) >> op_flags = NFCT_OF_SHOW_LAYER3; >> if (output_mask & _O_ID) >> op_flags |= NFCT_OF_ID; >> >> - nfct_snprintf(buf, 1024, ct, NFCT_T_UNKNOWN, op_type, op_flags); >> + nfct_snprintf(buf+len, 1024-len, ct, NFCT_T_UNKNOWN, op_type, >> op_flags); > > > It doesn't seem to matter here, but that looks buggy (combined > with the snprintf above). When the buffer size is exceed, snprintf > returns the amount of characters it *would have written* if > enough space was available. So when this really happens above, > you have a buffer overflow in the second snprintf. The string above has a fixed size and the buffer is big enough to print the flow entry, so the buffer overflow is very unlikely. Anyhow, I think that the following patch perform more strict and robust checkings regarding the buffer size. I hope that you like better :). -- "Los honestos son inadaptados sociales" -- Les Luthiers ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [conntrack-tools] XML output is invalid 2008-06-20 13:52 ` Pablo Neira Ayuso @ 2008-06-20 14:05 ` Patrick McHardy 2008-06-20 14:06 ` Pablo Neira Ayuso 1 sibling, 0 replies; 6+ messages in thread From: Patrick McHardy @ 2008-06-20 14:05 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Sen Haerens, Netfilter Development Mailinglist Pablo Neira Ayuso wrote: > Patrick McHardy wrote: >> Pablo Neira Ayuso wrote: >>> Does this patch help? >>> >>> >>> + if (output_mask & _O_XML) { >>> op_type = NFCT_O_XML; >>> + if (dump_xml_header_done) { >>> + dump_xml_header_done = 0; >>> + len = snprintf(buf, 1024, "<?xml version=\"1.0\"?>\n" >>> + "<conntrack>\n"); >>> + } >>> + } >>> if (output_mask & _O_EXT) >>> op_flags = NFCT_OF_SHOW_LAYER3; >>> if (output_mask & _O_ID) >>> op_flags |= NFCT_OF_ID; >>> >>> - nfct_snprintf(buf, 1024, ct, NFCT_T_UNKNOWN, op_type, op_flags); >>> + nfct_snprintf(buf+len, 1024-len, ct, NFCT_T_UNKNOWN, op_type, >>> op_flags); >> >> It doesn't seem to matter here, but that looks buggy (combined >> with the snprintf above). When the buffer size is exceed, snprintf >> returns the amount of characters it *would have written* if >> enough space was available. So when this really happens above, >> you have a buffer overflow in the second snprintf. > > The string above has a fixed size and the buffer is big enough to print > the flow entry, so the buffer overflow is very unlikely. Yeah, its more a correctness thing. And it might lead to problems later on. > Anyhow, I think that the following patch perform more strict and robust > checkings regarding the buffer size. I hope that you like better :). Probably, but its missing from this mail :) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [conntrack-tools] XML output is invalid 2008-06-20 13:52 ` Pablo Neira Ayuso 2008-06-20 14:05 ` Patrick McHardy @ 2008-06-20 14:06 ` Pablo Neira Ayuso 1 sibling, 0 replies; 6+ messages in thread From: Pablo Neira Ayuso @ 2008-06-20 14:06 UTC (permalink / raw) To: Patrick McHardy; +Cc: Sen Haerens, Netfilter Development Mailinglist [-- Attachment #1: Type: text/plain, Size: 1498 bytes --] Pablo Neira Ayuso wrote: > Patrick McHardy wrote: >> Pablo Neira Ayuso wrote: >>> Does this patch help? >>> >>> >>> + if (output_mask & _O_XML) { >>> op_type = NFCT_O_XML; >>> + if (dump_xml_header_done) { >>> + dump_xml_header_done = 0; >>> + len = snprintf(buf, 1024, "<?xml version=\"1.0\"?>\n" >>> + "<conntrack>\n"); >>> + } >>> + } >>> if (output_mask & _O_EXT) >>> op_flags = NFCT_OF_SHOW_LAYER3; >>> if (output_mask & _O_ID) >>> op_flags |= NFCT_OF_ID; >>> >>> - nfct_snprintf(buf, 1024, ct, NFCT_T_UNKNOWN, op_type, op_flags); >>> + nfct_snprintf(buf+len, 1024-len, ct, NFCT_T_UNKNOWN, op_type, >>> op_flags); >> >> It doesn't seem to matter here, but that looks buggy (combined >> with the snprintf above). When the buffer size is exceed, snprintf >> returns the amount of characters it *would have written* if >> enough space was available. So when this really happens above, >> you have a buffer overflow in the second snprintf. > > The string above has a fixed size and the buffer is big enough to print > the flow entry, so the buffer overflow is very unlikely. > > Anyhow, I think that the following patch perform more strict and robust > checkings regarding the buffer size. I hope that you like better :). Oops, I forgot the patch. I can also make another patch to check for other snprintf return values. -- "Los honestos son inadaptados sociales" -- Les Luthiers [-- Attachment #2: x --] [-- Type: text/plain, Size: 3473 bytes --] diff --git a/include/conntrack.h b/include/conntrack.h index 2e17475..dc30c13 100644 --- a/include/conntrack.h +++ b/include/conntrack.h @@ -189,4 +189,10 @@ extern void register_udp(void); extern void register_icmp(void); extern void register_icmpv6(void); +#define BUFFER_SIZE(ret, len, offset) \ + if (ret > len) \ + ret = len; \ + offset += ret; \ + len -= ret; + #endif diff --git a/src/conntrack.c b/src/conntrack.c index 25a3a57..f018c82 100644 --- a/src/conntrack.c +++ b/src/conntrack.c @@ -604,10 +604,16 @@ static int ignore_nat(const struct nf_conntrack *obj, } static int counter; +static int dump_xml_header_done = 1; static void __attribute__((noreturn)) event_sighandler(int s) { + if (dump_xml_header_done == 0) { + printf("</conntrack>\n"); + fflush(stdout); + } + fprintf(stderr, "%s v%s: ", PROGNAME, VERSION); fprintf(stderr, "%d flow events has been shown.\n", counter); nfct_close(cth); @@ -619,6 +625,7 @@ static int event_cb(enum nf_conntrack_msg_type type, void *data) { char buf[1024]; + int ret, offset = 0, len = 1024; struct nf_conntrack *obj = data; unsigned int op_type = NFCT_O_DEFAULT; unsigned int op_flags = 0; @@ -629,8 +636,20 @@ static int event_cb(enum nf_conntrack_msg_type type, if (options & CT_COMPARISON && !nfct_cmp(obj, ct, NFCT_CMP_ALL)) return NFCT_CB_CONTINUE; - if (output_mask & _O_XML) + if (output_mask & _O_XML) { op_type = NFCT_O_XML; + if (dump_xml_header_done) { + dump_xml_header_done = 0; + ret = snprintf(buf, len, "<?xml version=\"1.0\"?>\n" + "<conntrack>\n"); + if (ret == -1) { + fprintf(stderr, "evil! snprintf fails\n"); + return NFCT_CB_CONTINUE; + } + + BUFFER_SIZE(ret, len, offset); + } + } if (output_mask & _O_EXT) op_flags = NFCT_OF_SHOW_LAYER3; if (output_mask & _O_TMS) { @@ -644,7 +663,8 @@ static int event_cb(enum nf_conntrack_msg_type type, if (output_mask & _O_ID) op_flags |= NFCT_OF_ID; - nfct_snprintf(buf, 1024, ct, type, op_type, op_flags); + nfct_snprintf(buf+offset, len, ct, type, op_type, op_flags); + printf("%s\n", buf); fflush(stdout); @@ -658,6 +678,7 @@ static int dump_cb(enum nf_conntrack_msg_type type, void *data) { char buf[1024]; + int ret, offset = 0, len = 1024; struct nf_conntrack *obj = data; unsigned int op_type = NFCT_O_DEFAULT; unsigned int op_flags = 0; @@ -668,14 +689,26 @@ static int dump_cb(enum nf_conntrack_msg_type type, if (options & CT_COMPARISON && !nfct_cmp(obj, ct, NFCT_CMP_ALL)) return NFCT_CB_CONTINUE; - if (output_mask & _O_XML) + if (output_mask & _O_XML) { op_type = NFCT_O_XML; + if (dump_xml_header_done) { + dump_xml_header_done = 0; + ret = snprintf(buf, len, "<?xml version=\"1.0\"?>\n" + "<conntrack>\n"); + if (ret == -1) { + fprintf(stderr, "evil! snprintf fails\n"); + return NFCT_CB_CONTINUE; + } + + BUFFER_SIZE(ret, len, offset); + } + } if (output_mask & _O_EXT) op_flags = NFCT_OF_SHOW_LAYER3; if (output_mask & _O_ID) op_flags |= NFCT_OF_ID; - nfct_snprintf(buf, 1024, ct, NFCT_T_UNKNOWN, op_type, op_flags); + nfct_snprintf(buf+offset, len, ct, NFCT_T_UNKNOWN, op_type, op_flags); printf("%s\n", buf); counter++; @@ -1129,6 +1162,11 @@ int main(int argc, char *argv[]) else res = nfct_query(cth, NFCT_Q_DUMP, &family); + if (dump_xml_header_done == 0) { + printf("</conntrack>\n"); + fflush(stdout); + } + nfct_close(cth); break; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [conntrack-tools] XML output is invalid 2008-06-20 13:24 ` [conntrack-tools] XML output is invalid Pablo Neira Ayuso 2008-06-20 13:30 ` Patrick McHardy @ 2008-06-20 14:03 ` Jan Engelhardt 1 sibling, 0 replies; 6+ messages in thread From: Jan Engelhardt @ 2008-06-20 14:03 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Sen Haerens, Netfilter Development Mailinglist On Friday 2008-06-20 15:24, Pablo Neira Ayuso wrote: >> >> * The XML declaration is missing: <?xml version="1.0"?> >> * There can only be one root element. We have to wrap all the flow >> elements into a single root element: >> <conntrack> >> <flow>[...]</flow> >> <flow>[...]</flow> >> [...] >> </conntrack> >> >> My version: conntrack v0.9.7 (conntrack-tools) > >Does this patch help? Also specify the encoding: <?xml version="1.0" encoding="utf-8" ?> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-06-20 14:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <F7825499-A8F5-4F41-B3DD-2E13807954E2@rotsen.be>
2008-06-20 13:24 ` [conntrack-tools] XML output is invalid Pablo Neira Ayuso
2008-06-20 13:30 ` Patrick McHardy
2008-06-20 13:52 ` Pablo Neira Ayuso
2008-06-20 14:05 ` Patrick McHardy
2008-06-20 14:06 ` Pablo Neira Ayuso
2008-06-20 14:03 ` Jan Engelhardt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.