From: Robert R. <rob...@am...> - 2008-11-21 19:06:31
|
Signed-off-by: Robert Richter <rob...@am...> --- ChangeLog | 8 ++ libop/op_xml_events.c | 65 ++++++++------- libop/op_xml_out.c | 197 +++++++++++++++++++++++++++------------------- libop/op_xml_out.h | 10 +- libutil++/xml_output.cpp | 18 ++--- 5 files changed, 170 insertions(+), 128 deletions(-) diff --git a/ChangeLog b/ChangeLog index e7a858c..a13b131 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2008-11-21 Robert Richter <rob...@am...> + + * libop/op_xml_events.c + * libop/op_xml_out.c + * libop/op_xml_out.h + * libutil++/xml_output.cpputils/opcontrol: buffer overflow fixes + in xml generator + 2008-11-17 Robert Richter <rob...@am...> * utils/opcontrol: restore parameter dump of --start and diff --git a/libop/op_xml_events.c b/libop/op_xml_events.c index 5b9ac7d..63f21d2 100644 --- a/libop/op_xml_events.c +++ b/libop/op_xml_events.c @@ -17,37 +17,37 @@ static op_cpu cpu_type; #define MAX_BUFFER 4096 +static char buffer[MAX_BUFFER]; + void open_xml_events(char const * title, char const * doc, op_cpu the_cpu_type) { char const * schema_version = "1.0"; - char buffer[MAX_BUFFER]; buffer[0] = '\0'; cpu_type = the_cpu_type; - open_xml_element(HELP_EVENTS, 0, buffer); - open_xml_element(HELP_HEADER, 1, buffer); - init_xml_str_attr(HELP_TITLE, title, buffer); - init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer); - init_xml_str_attr(HELP_DOC, doc, buffer); - close_xml_element(NONE, 0, buffer); + open_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); + open_xml_element(HELP_HEADER, 1, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_TITLE, title, buffer, MAX_BUFFER); + init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_DOC, doc, buffer, MAX_BUFFER); + close_xml_element(NONE, 0, buffer, MAX_BUFFER); printf("%s", buffer); } void close_xml_events(void) { - char buffer[MAX_BUFFER]; - buffer[0] = '\0'; - close_xml_element(HELP_EVENTS, 0, buffer); + close_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); printf("%s", buffer); } -static void xml_do_arch_specific_event_help(struct op_event const * event, - char * buffer) +static void xml_do_arch_specific_event_help(struct op_event const *event, + char *buffer, size_t size) { switch (cpu_type) { case CPU_PPC64_CELL: - init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer); + init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer, + size); break; default: break; @@ -60,34 +60,39 @@ void xml_help_for_event(struct op_event const * event) uint i; int nr_counters; int has_nested = strcmp(event->unit->name, "zero"); - char buffer[MAX_BUFFER]; buffer[0] = '\0'; - open_xml_element(HELP_EVENT, 1, buffer); - init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer); - xml_do_arch_specific_event_help(event, buffer); - init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer); + open_xml_element(HELP_EVENT, 1, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer, MAX_BUFFER); + xml_do_arch_specific_event_help(event, buffer, MAX_BUFFER); + init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer, MAX_BUFFER); nr_counters = op_get_nr_counters(cpu_type); - init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer); - init_xml_int_attr(HELP_MIN_COUNT, event->min_count, buffer); + init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer, + MAX_BUFFER); + init_xml_int_attr(HELP_MIN_COUNT, event->min_count, + buffer, MAX_BUFFER); if (has_nested) { - close_xml_element(NONE, 1, buffer); - open_xml_element(HELP_UNIT_MASKS, 1, buffer); - init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, buffer); - close_xml_element(NONE, 1, buffer); + close_xml_element(NONE, 1, buffer, MAX_BUFFER); + open_xml_element(HELP_UNIT_MASKS, 1, buffer, MAX_BUFFER); + init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, + buffer, MAX_BUFFER); + close_xml_element(NONE, 1, buffer, MAX_BUFFER); for (i = 0; i < event->unit->num; i++) { - open_xml_element(HELP_UNIT_MASK, 1, buffer); + open_xml_element(HELP_UNIT_MASK, 1, buffer, MAX_BUFFER); init_xml_int_attr(HELP_UNIT_MASK_VALUE, - event->unit->um[i].value, buffer); + event->unit->um[i].value, + buffer, MAX_BUFFER); init_xml_str_attr(HELP_UNIT_MASK_DESC, - event->unit->um[i].desc, buffer); - close_xml_element(NONE, 0, buffer); + event->unit->um[i].desc, + buffer, MAX_BUFFER); + close_xml_element(NONE, 0, buffer, MAX_BUFFER); } - close_xml_element(HELP_UNIT_MASKS, 0, buffer); + close_xml_element(HELP_UNIT_MASKS, 0, buffer, MAX_BUFFER); } - close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, buffer); + close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, + buffer, MAX_BUFFER); printf("%s", buffer); } diff --git a/libop/op_xml_out.c b/libop/op_xml_out.c index d779c45..7cf0abb 100644 --- a/libop/op_xml_out.c +++ b/libop/op_xml_out.c @@ -91,143 +91,176 @@ char const * xml_tag_name(tag_t tag) } -void open_xml_element(tag_t tag, int with_attrs, char * buffer) +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t max) { - char const * tag_name = xml_tag_name(tag); - unsigned int const max_len = strlen(tag_name) + 3; - char tmp_buf[MAX_BUF_LEN]; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr,"Warning: open_xml_element: buffer overflow %d\n", max_len); + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (snprintf(tmp_buf, sizeof(tmp_buf), "<%s%s", tag_name, - (with_attrs ? " " : ">\n")) < 0) { + ret = snprintf(buf, size, "<%s%s", xml_tag_name(tag), + (with_attrs ? " " : ">\n")); + + if (ret < 0 || ret >= size) { fprintf(stderr,"open_xml_element: snprintf failed\n"); exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -void close_xml_element(tag_t tag, int has_nested, char * buffer) +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t max) { - char const * tag_name = xml_tag_name(tag); - unsigned int const max_len = strlen(tag_name) + 3; - char tmp_buf[MAX_BUF_LEN]; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr,"Warning: close_xml_element: buffer overflow %d\n", max_len); + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (tag == NONE) { - if (snprintf(tmp_buf, sizeof(tmp_buf), "%s\n", (has_nested ? ">" : "/>")) < 0) { - fprintf(stderr, "close_xml_element: snprintf failed\n"); - exit(EXIT_FAILURE); - } - } else { - if (snprintf(tmp_buf, sizeof(tmp_buf), "</%s>\n", tag_name) < 0) { - fprintf(stderr, "close_xml_element: snprintf failed\n"); - exit(EXIT_FAILURE); - } + if (tag == NONE) + ret = snprintf(buf, size, "%s\n", (has_nested ? ">" : "/>")); + else + ret = snprintf(buf, size, "</%s>\n", xml_tag_name(tag)); + + if (ret < 0 || ret >= size) { + fprintf(stderr, "close_xml_element: snprintf failed\n"); + exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -void init_xml_int_attr(tag_t attr, int value, char * buffer) +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t max) { - char const * attr_name = xml_tag_name(attr); - char tmp_buf[MAX_BUF_LEN]; - unsigned int const max_len = strlen(attr_name) + 50; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) { - fprintf(stderr, - "Warning: init_xml_int_attr: buffer overflow %d\n", max_len); - } + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; + ret = snprintf(buf, size, " %s=\"%d\"", xml_tag_name(attr), value); - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%d\"", attr_name, value) < 0) { + if (ret < 0 || ret >= size) { fprintf(stderr,"init_xml_int_attr: snprintf failed\n"); exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -void init_xml_dbl_attr(tag_t attr, double value, char * buffer) +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t max) { - char const * attr_name = xml_tag_name(attr); - unsigned int const max_len = strlen(attr_name) + 50; - char tmp_buf[MAX_BUF_LEN]; + char *buf; + int size, ret; + + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr, "Warning: init_xml_dbl_attr: buffer overflow %d\n", max_len); + ret = snprintf(buf, size, " %s=\"%.2f\"", xml_tag_name(attr), value); - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%.2f\"", attr_name, value) < 0) { + if (ret < 0 || ret >= size) { fprintf(stderr, "init_xml_dbl_attr: snprintf failed\n"); exit(EXIT_FAILURE); } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); } -static char * xml_quote(char const * str, char * quote_buf) +static void xml_quote(char const *str, char *buffer, size_t max) { - int i; - int pos = 0; - int len = strlen(str); + char *buf; + char *quote; + char *pos = (char*)str; + size_t size; + int ret; - - quote_buf[pos++] = '"'; + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - for (i = 0; i < len; i++) { - if (pos >= MAX_BUF_LEN - 10) { - fprintf(stderr,"quote_str: buffer overflow %d\n", pos); - exit(EXIT_FAILURE); - } + if (size < strlen(pos) + 2) + goto Error; - switch(str[i]) { + *buf = '"'; + buf++; + size--; + + while (*pos) { + switch(*pos) { case '&': - strncpy(quote_buf + pos, "&", 5); - pos += 5; + quote = "&"; break; case '<': - strncpy(quote_buf + pos, "<", 4); - pos += 4; + quote = "<"; break; case '>': - strncpy(quote_buf + pos, ">", 4); - pos += 4; + quote = ">"; break; case '"': - strncpy(quote_buf + pos, """, 6); - pos += 6; + quote = """; break; default: - quote_buf[pos++] = str[i]; - break; + *buf = *pos; + pos++; + buf++; + size--; + continue; } + + pos++; + ret = snprintf(buf, size, "%s", quote); + if (ret < 0 || ret >= (int)size) + goto Error; + buf += ret; + size -= ret; + if (size < strlen(pos)) + goto Error; } - quote_buf[pos++] = '"'; - quote_buf[pos++] = '\0'; - return quote_buf; + if (!size) + goto Error; + + *buf = '"'; + buf++; + *buf = '\0'; + + return; + +Error: + fprintf(stderr,"quote_str: buffer overflow\n"); + exit(EXIT_FAILURE); } -void init_xml_str_attr(tag_t attr, char const * str, char * buffer) +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t max) { - char tmp_buf[MAX_BUF_LEN]; - char quote_buf[MAX_BUF_LEN]; - char const * attr_name = xml_tag_name(attr); - char const * quote_str = xml_quote(str, quote_buf); - const unsigned int max_len = strlen(attr_name) + strlen(quote_str) + 10; + char *buf; + int size, ret; - if (max_len >= sizeof(tmp_buf)) - fprintf(stderr, "Warning: init_xml_str_attr: buffer overflow %d\n", max_len); + buffer[max - 1] = '\0'; + size = strlen(buffer); + buf = &buffer[size]; + size = max - 1 - size; - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=""%s""", attr_name, quote_str) < 0) { - fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); - exit(EXIT_FAILURE); - } - strncat(buffer, tmp_buf, sizeof(tmp_buf)); + ret = snprintf(buf, size, " %s=", xml_tag_name(attr)); + if (ret < 0 || ret >= size) + goto Error; + + buf += ret; + size -= ret; + + if (!size) + goto Error; + + xml_quote(str, buf, size); + return; +Error: + fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); + exit(EXIT_FAILURE); } diff --git a/libop/op_xml_out.h b/libop/op_xml_out.h index 52e8d8f..ae0fea5 100644 --- a/libop/op_xml_out.h +++ b/libop/op_xml_out.h @@ -59,11 +59,11 @@ typedef enum { } tag_t; char const * xml_tag_name(tag_t tag); -void open_xml_element(tag_t tag, int with_attrs, char * result); -void close_xml_element(tag_t tag, int has_nested, char * result); -void init_xml_int_attr(tag_t attr, int value, char * result); -void init_xml_dbl_attr(tag_t attr, double value, char * result); -void init_xml_str_attr(tag_t attr, char const * str, char * result); +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t size); +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t size); +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t size); +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t size); +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t size); #ifdef __cplusplus } diff --git a/libutil++/xml_output.cpp b/libutil++/xml_output.cpp index 0f0b189..4ac9a1b 100644 --- a/libutil++/xml_output.cpp +++ b/libutil++/xml_output.cpp @@ -16,7 +16,8 @@ using namespace std; -#define MAX_XML_BUF 1024 +#define MAX_XML_BUF 4096 +static char buf[MAX_XML_BUF]; string tag_name(tag_t tag) { @@ -29,10 +30,9 @@ string tag_name(tag_t tag) string open_element(tag_t tag, bool with_attrs) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - open_xml_element(tag, with_attrs, buf); + open_xml_element(tag, with_attrs, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -41,10 +41,9 @@ string open_element(tag_t tag, bool with_attrs) string close_element(tag_t tag, bool has_nested) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - close_xml_element(tag, has_nested, buf); + close_xml_element(tag, has_nested, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -53,10 +52,9 @@ string close_element(tag_t tag, bool has_nested) string init_attr(tag_t attr, size_t value) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - init_xml_int_attr(attr, value, buf); + init_xml_int_attr(attr, value, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -65,10 +63,9 @@ string init_attr(tag_t attr, size_t value) string init_attr(tag_t attr, double value) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - init_xml_dbl_attr(attr, value, buf); + init_xml_dbl_attr(attr, value, buf, MAX_XML_BUF); out << buf; return out.str(); } @@ -77,10 +74,9 @@ string init_attr(tag_t attr, double value) string init_attr(tag_t attr, string const & str) { ostringstream out; - char buf[MAX_XML_BUF]; buf[0] = '\0'; - init_xml_str_attr(attr, str.c_str(), buf); + init_xml_str_attr(attr, str.c_str(), buf, MAX_XML_BUF); out << buf; return out.str(); } -- 1.6.0.1 |
From: Maynard J. <may...@us...> - 2008-12-23 16:51:58
|
Robert Richter wrote: > Signed-off-by: Robert Richter <rob...@am...> Robert, I sincerely apologize for not replying to this patch before now. The posting got buried in my inbox, and I didn't see it until today (while doing some cleanup during holiday quiet time). Does this patch fix some vulnerability? I'm not seeing where a user might actually exploit the existing code to cause a buffer overflow. Can you point this out? Thanks. -Maynard > --- > ChangeLog | 8 ++ > libop/op_xml_events.c | 65 ++++++++------- > libop/op_xml_out.c | 197 +++++++++++++++++++++++++++------------------- > libop/op_xml_out.h | 10 +- > libutil++/xml_output.cpp | 18 ++--- > 5 files changed, 170 insertions(+), 128 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index e7a858c..a13b131 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,11 @@ > +2008-11-21 Robert Richter <rob...@am...> > + > + * libop/op_xml_events.c > + * libop/op_xml_out.c > + * libop/op_xml_out.h > + * libutil++/xml_output.cpputils/opcontrol: buffer overflow fixes > + in xml generator > + > 2008-11-17 Robert Richter <rob...@am...> > > * utils/opcontrol: restore parameter dump of --start and > diff --git a/libop/op_xml_events.c b/libop/op_xml_events.c > index 5b9ac7d..63f21d2 100644 > --- a/libop/op_xml_events.c > +++ b/libop/op_xml_events.c > @@ -17,37 +17,37 @@ > > static op_cpu cpu_type; > #define MAX_BUFFER 4096 > +static char buffer[MAX_BUFFER]; > + > void open_xml_events(char const * title, char const * doc, op_cpu the_cpu_type) > { > char const * schema_version = "1.0"; > - char buffer[MAX_BUFFER]; > > buffer[0] = '\0'; > cpu_type = the_cpu_type; > - open_xml_element(HELP_EVENTS, 0, buffer); > - open_xml_element(HELP_HEADER, 1, buffer); > - init_xml_str_attr(HELP_TITLE, title, buffer); > - init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer); > - init_xml_str_attr(HELP_DOC, doc, buffer); > - close_xml_element(NONE, 0, buffer); > + open_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); > + open_xml_element(HELP_HEADER, 1, buffer, MAX_BUFFER); > + init_xml_str_attr(HELP_TITLE, title, buffer, MAX_BUFFER); > + init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer, MAX_BUFFER); > + init_xml_str_attr(HELP_DOC, doc, buffer, MAX_BUFFER); > + close_xml_element(NONE, 0, buffer, MAX_BUFFER); > printf("%s", buffer); > } > > void close_xml_events(void) > { > - char buffer[MAX_BUFFER]; > - > buffer[0] = '\0'; > - close_xml_element(HELP_EVENTS, 0, buffer); > + close_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); > printf("%s", buffer); > } > > -static void xml_do_arch_specific_event_help(struct op_event const * event, > - char * buffer) > +static void xml_do_arch_specific_event_help(struct op_event const *event, > + char *buffer, size_t size) > { > switch (cpu_type) { > case CPU_PPC64_CELL: > - init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer); > + init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer, > + size); > break; > default: > break; > @@ -60,34 +60,39 @@ void xml_help_for_event(struct op_event const * event) > uint i; > int nr_counters; > int has_nested = strcmp(event->unit->name, "zero"); > - char buffer[MAX_BUFFER]; > > buffer[0] = '\0'; > - open_xml_element(HELP_EVENT, 1, buffer); > - init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer); > - xml_do_arch_specific_event_help(event, buffer); > - init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer); > + open_xml_element(HELP_EVENT, 1, buffer, MAX_BUFFER); > + init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer, MAX_BUFFER); > + xml_do_arch_specific_event_help(event, buffer, MAX_BUFFER); > + init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer, MAX_BUFFER); > > nr_counters = op_get_nr_counters(cpu_type); > - init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer); > - init_xml_int_attr(HELP_MIN_COUNT, event->min_count, buffer); > + init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer, > + MAX_BUFFER); > + init_xml_int_attr(HELP_MIN_COUNT, event->min_count, > + buffer, MAX_BUFFER); > > if (has_nested) { > - close_xml_element(NONE, 1, buffer); > - open_xml_element(HELP_UNIT_MASKS, 1, buffer); > - init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, buffer); > - close_xml_element(NONE, 1, buffer); > + close_xml_element(NONE, 1, buffer, MAX_BUFFER); > + open_xml_element(HELP_UNIT_MASKS, 1, buffer, MAX_BUFFER); > + init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, > + buffer, MAX_BUFFER); > + close_xml_element(NONE, 1, buffer, MAX_BUFFER); > for (i = 0; i < event->unit->num; i++) { > - open_xml_element(HELP_UNIT_MASK, 1, buffer); > + open_xml_element(HELP_UNIT_MASK, 1, buffer, MAX_BUFFER); > init_xml_int_attr(HELP_UNIT_MASK_VALUE, > - event->unit->um[i].value, buffer); > + event->unit->um[i].value, > + buffer, MAX_BUFFER); > init_xml_str_attr(HELP_UNIT_MASK_DESC, > - event->unit->um[i].desc, buffer); > - close_xml_element(NONE, 0, buffer); > + event->unit->um[i].desc, > + buffer, MAX_BUFFER); > + close_xml_element(NONE, 0, buffer, MAX_BUFFER); > } > - close_xml_element(HELP_UNIT_MASKS, 0, buffer); > + close_xml_element(HELP_UNIT_MASKS, 0, buffer, MAX_BUFFER); > } > - close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, buffer); > + close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, > + buffer, MAX_BUFFER); > printf("%s", buffer); > } > > diff --git a/libop/op_xml_out.c b/libop/op_xml_out.c > index d779c45..7cf0abb 100644 > --- a/libop/op_xml_out.c > +++ b/libop/op_xml_out.c > @@ -91,143 +91,176 @@ char const * xml_tag_name(tag_t tag) > } > > > -void open_xml_element(tag_t tag, int with_attrs, char * buffer) > +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t max) > { > - char const * tag_name = xml_tag_name(tag); > - unsigned int const max_len = strlen(tag_name) + 3; > - char tmp_buf[MAX_BUF_LEN]; > + char *buf; > + int size, ret; > > - if (max_len >= sizeof(tmp_buf)) > - fprintf(stderr,"Warning: open_xml_element: buffer overflow %d\n", max_len); > + buffer[max - 1] = '\0'; > + size = strlen(buffer); > + buf = &buffer[size]; > + size = max - 1 - size; > > - if (snprintf(tmp_buf, sizeof(tmp_buf), "<%s%s", tag_name, > - (with_attrs ? " " : ">\n")) < 0) { > + ret = snprintf(buf, size, "<%s%s", xml_tag_name(tag), > + (with_attrs ? " " : ">\n")); > + > + if (ret < 0 || ret >= size) { > fprintf(stderr,"open_xml_element: snprintf failed\n"); > exit(EXIT_FAILURE); > } > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > } > > > -void close_xml_element(tag_t tag, int has_nested, char * buffer) > +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t max) > { > - char const * tag_name = xml_tag_name(tag); > - unsigned int const max_len = strlen(tag_name) + 3; > - char tmp_buf[MAX_BUF_LEN]; > + char *buf; > + int size, ret; > > - if (max_len >= sizeof(tmp_buf)) > - fprintf(stderr,"Warning: close_xml_element: buffer overflow %d\n", max_len); > + buffer[max - 1] = '\0'; > + size = strlen(buffer); > + buf = &buffer[size]; > + size = max - 1 - size; > > - if (tag == NONE) { > - if (snprintf(tmp_buf, sizeof(tmp_buf), "%s\n", (has_nested ? ">" : "/>")) < 0) { > - fprintf(stderr, "close_xml_element: snprintf failed\n"); > - exit(EXIT_FAILURE); > - } > - } else { > - if (snprintf(tmp_buf, sizeof(tmp_buf), "</%s>\n", tag_name) < 0) { > - fprintf(stderr, "close_xml_element: snprintf failed\n"); > - exit(EXIT_FAILURE); > - } > + if (tag == NONE) > + ret = snprintf(buf, size, "%s\n", (has_nested ? ">" : "/>")); > + else > + ret = snprintf(buf, size, "</%s>\n", xml_tag_name(tag)); > + > + if (ret < 0 || ret >= size) { > + fprintf(stderr, "close_xml_element: snprintf failed\n"); > + exit(EXIT_FAILURE); > } > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > } > > > -void init_xml_int_attr(tag_t attr, int value, char * buffer) > +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t max) > { > - char const * attr_name = xml_tag_name(attr); > - char tmp_buf[MAX_BUF_LEN]; > - unsigned int const max_len = strlen(attr_name) + 50; > + char *buf; > + int size, ret; > > - if (max_len >= sizeof(tmp_buf)) { > - fprintf(stderr, > - "Warning: init_xml_int_attr: buffer overflow %d\n", max_len); > - } > + buffer[max - 1] = '\0'; > + size = strlen(buffer); > + buf = &buffer[size]; > + size = max - 1 - size; > > + ret = snprintf(buf, size, " %s=\"%d\"", xml_tag_name(attr), value); > > - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%d\"", attr_name, value) < 0) { > + if (ret < 0 || ret >= size) { > fprintf(stderr,"init_xml_int_attr: snprintf failed\n"); > exit(EXIT_FAILURE); > } > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > } > > > -void init_xml_dbl_attr(tag_t attr, double value, char * buffer) > +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t max) > { > - char const * attr_name = xml_tag_name(attr); > - unsigned int const max_len = strlen(attr_name) + 50; > - char tmp_buf[MAX_BUF_LEN]; > + char *buf; > + int size, ret; > + > + buffer[max - 1] = '\0'; > + size = strlen(buffer); > + buf = &buffer[size]; > + size = max - 1 - size; > > - if (max_len >= sizeof(tmp_buf)) > - fprintf(stderr, "Warning: init_xml_dbl_attr: buffer overflow %d\n", max_len); > + ret = snprintf(buf, size, " %s=\"%.2f\"", xml_tag_name(attr), value); > > - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%.2f\"", attr_name, value) < 0) { > + if (ret < 0 || ret >= size) { > fprintf(stderr, "init_xml_dbl_attr: snprintf failed\n"); > exit(EXIT_FAILURE); > } > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > } > > > -static char * xml_quote(char const * str, char * quote_buf) > +static void xml_quote(char const *str, char *buffer, size_t max) > { > - int i; > - int pos = 0; > - int len = strlen(str); > + char *buf; > + char *quote; > + char *pos = (char*)str; > + size_t size; > + int ret; > > - > - quote_buf[pos++] = '"'; > + buffer[max - 1] = '\0'; > + size = strlen(buffer); > + buf = &buffer[size]; > + size = max - 1 - size; > > - for (i = 0; i < len; i++) { > - if (pos >= MAX_BUF_LEN - 10) { > - fprintf(stderr,"quote_str: buffer overflow %d\n", pos); > - exit(EXIT_FAILURE); > - } > + if (size < strlen(pos) + 2) > + goto Error; > > - switch(str[i]) { > + *buf = '"'; > + buf++; > + size--; > + > + while (*pos) { > + switch(*pos) { > case '&': > - strncpy(quote_buf + pos, "&", 5); > - pos += 5; > + quote = "&"; > break; > case '<': > - strncpy(quote_buf + pos, "<", 4); > - pos += 4; > + quote = "<"; > break; > case '>': > - strncpy(quote_buf + pos, ">", 4); > - pos += 4; > + quote = ">"; > break; > case '"': > - strncpy(quote_buf + pos, """, 6); > - pos += 6; > + quote = """; > break; > default: > - quote_buf[pos++] = str[i]; > - break; > + *buf = *pos; > + pos++; > + buf++; > + size--; > + continue; > } > + > + pos++; > + ret = snprintf(buf, size, "%s", quote); > + if (ret < 0 || ret >= (int)size) > + goto Error; > + buf += ret; > + size -= ret; > + if (size < strlen(pos)) > + goto Error; > } > > - quote_buf[pos++] = '"'; > - quote_buf[pos++] = '\0'; > - return quote_buf; > + if (!size) > + goto Error; > + > + *buf = '"'; > + buf++; > + *buf = '\0'; > + > + return; > + > +Error: > + fprintf(stderr,"quote_str: buffer overflow\n"); > + exit(EXIT_FAILURE); > } > > > -void init_xml_str_attr(tag_t attr, char const * str, char * buffer) > +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t max) > { > - char tmp_buf[MAX_BUF_LEN]; > - char quote_buf[MAX_BUF_LEN]; > - char const * attr_name = xml_tag_name(attr); > - char const * quote_str = xml_quote(str, quote_buf); > - const unsigned int max_len = strlen(attr_name) + strlen(quote_str) + 10; > + char *buf; > + int size, ret; > > - if (max_len >= sizeof(tmp_buf)) > - fprintf(stderr, "Warning: init_xml_str_attr: buffer overflow %d\n", max_len); > + buffer[max - 1] = '\0'; > + size = strlen(buffer); > + buf = &buffer[size]; > + size = max - 1 - size; > > - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=""%s""", attr_name, quote_str) < 0) { > - fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); > - exit(EXIT_FAILURE); > - } > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > + ret = snprintf(buf, size, " %s=", xml_tag_name(attr)); > + if (ret < 0 || ret >= size) > + goto Error; > + > + buf += ret; > + size -= ret; > + > + if (!size) > + goto Error; > + > + xml_quote(str, buf, size); > + return; > +Error: > + fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); > + exit(EXIT_FAILURE); > } > diff --git a/libop/op_xml_out.h b/libop/op_xml_out.h > index 52e8d8f..ae0fea5 100644 > --- a/libop/op_xml_out.h > +++ b/libop/op_xml_out.h > @@ -59,11 +59,11 @@ typedef enum { > } tag_t; > > char const * xml_tag_name(tag_t tag); > -void open_xml_element(tag_t tag, int with_attrs, char * result); > -void close_xml_element(tag_t tag, int has_nested, char * result); > -void init_xml_int_attr(tag_t attr, int value, char * result); > -void init_xml_dbl_attr(tag_t attr, double value, char * result); > -void init_xml_str_attr(tag_t attr, char const * str, char * result); > +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t size); > +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t size); > +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t size); > +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t size); > +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t size); > > #ifdef __cplusplus > } > diff --git a/libutil++/xml_output.cpp b/libutil++/xml_output.cpp > index 0f0b189..4ac9a1b 100644 > --- a/libutil++/xml_output.cpp > +++ b/libutil++/xml_output.cpp > @@ -16,7 +16,8 @@ > > using namespace std; > > -#define MAX_XML_BUF 1024 > +#define MAX_XML_BUF 4096 > +static char buf[MAX_XML_BUF]; > > string tag_name(tag_t tag) > { > @@ -29,10 +30,9 @@ string tag_name(tag_t tag) > string open_element(tag_t tag, bool with_attrs) > { > ostringstream out; > - char buf[MAX_XML_BUF]; > > buf[0] = '\0'; > - open_xml_element(tag, with_attrs, buf); > + open_xml_element(tag, with_attrs, buf, MAX_XML_BUF); > out << buf; > return out.str(); > } > @@ -41,10 +41,9 @@ string open_element(tag_t tag, bool with_attrs) > string close_element(tag_t tag, bool has_nested) > { > ostringstream out; > - char buf[MAX_XML_BUF]; > > buf[0] = '\0'; > - close_xml_element(tag, has_nested, buf); > + close_xml_element(tag, has_nested, buf, MAX_XML_BUF); > out << buf; > return out.str(); > } > @@ -53,10 +52,9 @@ string close_element(tag_t tag, bool has_nested) > string init_attr(tag_t attr, size_t value) > { > ostringstream out; > - char buf[MAX_XML_BUF]; > > buf[0] = '\0'; > - init_xml_int_attr(attr, value, buf); > + init_xml_int_attr(attr, value, buf, MAX_XML_BUF); > out << buf; > return out.str(); > } > @@ -65,10 +63,9 @@ string init_attr(tag_t attr, size_t value) > string init_attr(tag_t attr, double value) > { > ostringstream out; > - char buf[MAX_XML_BUF]; > > buf[0] = '\0'; > - init_xml_dbl_attr(attr, value, buf); > + init_xml_dbl_attr(attr, value, buf, MAX_XML_BUF); > out << buf; > return out.str(); > } > @@ -77,10 +74,9 @@ string init_attr(tag_t attr, double value) > string init_attr(tag_t attr, string const & str) > { > ostringstream out; > - char buf[MAX_XML_BUF]; > > buf[0] = '\0'; > - init_xml_str_attr(attr, str.c_str(), buf); > + init_xml_str_attr(attr, str.c_str(), buf, MAX_XML_BUF); > out << buf; > return out.str(); > } |
From: Robert R. <rob...@am...> - 2009-01-20 17:07:18
|
On 23.12.08 10:51:53, Maynard Johnson wrote: > I sincerely apologize for not replying to this patch before now. > The posting got buried in my inbox, and I didn't see it until today > (while doing some cleanup during holiday quiet time). Does this > patch fix some vulnerability? I'm not seeing where a user might > actually exploit the existing code to cause a buffer overflow. Can > you point this out? Maynard, I experienced segfaults when running opreport during my oprofile development. I got some data that was producing large xml tags causing a buffer overflow. Looking at the code this behaviour was clear. The function interface is broken by design: -static void xml_do_arch_specific_event_help(struct op_event const * event, - char * buffer) +static void xml_do_arch_specific_event_help(struct op_event const *event, + char *buffer, size_t size) It requires a buffer to write to without providing the size of the buffer. Thus it is not possible to avoid buffer overflows since the size of xml tags may vary and depends on the input data. This patch fixes this. Unfortunately I am not able to reproduce data again that triggers the bug, but this patch clearly improves code quality. I think it should be applied anyway. Thanks, -Robert > > Thanks. > -Maynard > > --- > > ChangeLog | 8 ++ > > libop/op_xml_events.c | 65 ++++++++------- > > libop/op_xml_out.c | 197 +++++++++++++++++++++++++++------------------- > > libop/op_xml_out.h | 10 +- > > libutil++/xml_output.cpp | 18 ++--- > > 5 files changed, 170 insertions(+), 128 deletions(-) > > > > diff --git a/ChangeLog b/ChangeLog > > index e7a858c..a13b131 100644 > > --- a/ChangeLog > > +++ b/ChangeLog > > @@ -1,3 +1,11 @@ > > +2008-11-21 Robert Richter <rob...@am...> > > + > > + * libop/op_xml_events.c > > + * libop/op_xml_out.c > > + * libop/op_xml_out.h > > + * libutil++/xml_output.cpputils/opcontrol: buffer overflow fixes > > + in xml generator > > + > > 2008-11-17 Robert Richter <rob...@am...> > > > > * utils/opcontrol: restore parameter dump of --start and > > diff --git a/libop/op_xml_events.c b/libop/op_xml_events.c > > index 5b9ac7d..63f21d2 100644 > > --- a/libop/op_xml_events.c > > +++ b/libop/op_xml_events.c > > @@ -17,37 +17,37 @@ > > > > static op_cpu cpu_type; > > #define MAX_BUFFER 4096 > > +static char buffer[MAX_BUFFER]; > > + > > void open_xml_events(char const * title, char const * doc, op_cpu the_cpu_type) > > { > > char const * schema_version = "1.0"; > > - char buffer[MAX_BUFFER]; > > > > buffer[0] = '\0'; > > cpu_type = the_cpu_type; > > - open_xml_element(HELP_EVENTS, 0, buffer); > > - open_xml_element(HELP_HEADER, 1, buffer); > > - init_xml_str_attr(HELP_TITLE, title, buffer); > > - init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer); > > - init_xml_str_attr(HELP_DOC, doc, buffer); > > - close_xml_element(NONE, 0, buffer); > > + open_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); > > + open_xml_element(HELP_HEADER, 1, buffer, MAX_BUFFER); > > + init_xml_str_attr(HELP_TITLE, title, buffer, MAX_BUFFER); > > + init_xml_str_attr(SCHEMA_VERSION, schema_version, buffer, MAX_BUFFER); > > + init_xml_str_attr(HELP_DOC, doc, buffer, MAX_BUFFER); > > + close_xml_element(NONE, 0, buffer, MAX_BUFFER); > > printf("%s", buffer); > > } > > > > void close_xml_events(void) > > { > > - char buffer[MAX_BUFFER]; > > - > > buffer[0] = '\0'; > > - close_xml_element(HELP_EVENTS, 0, buffer); > > + close_xml_element(HELP_EVENTS, 0, buffer, MAX_BUFFER); > > printf("%s", buffer); > > } > > > > -static void xml_do_arch_specific_event_help(struct op_event const * event, > > - char * buffer) > > +static void xml_do_arch_specific_event_help(struct op_event const *event, > > + char *buffer, size_t size) > > { > > switch (cpu_type) { > > case CPU_PPC64_CELL: > > - init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer); > > + init_xml_int_attr(HELP_EVENT_GROUP, event->val / 100, buffer, > > + size); > > break; > > default: > > break; > > @@ -60,34 +60,39 @@ void xml_help_for_event(struct op_event const * event) > > uint i; > > int nr_counters; > > int has_nested = strcmp(event->unit->name, "zero"); > > - char buffer[MAX_BUFFER]; > > > > buffer[0] = '\0'; > > - open_xml_element(HELP_EVENT, 1, buffer); > > - init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer); > > - xml_do_arch_specific_event_help(event, buffer); > > - init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer); > > + open_xml_element(HELP_EVENT, 1, buffer, MAX_BUFFER); > > + init_xml_str_attr(HELP_EVENT_NAME, event->name, buffer, MAX_BUFFER); > > + xml_do_arch_specific_event_help(event, buffer, MAX_BUFFER); > > + init_xml_str_attr(HELP_EVENT_DESC, event->desc, buffer, MAX_BUFFER); > > > > nr_counters = op_get_nr_counters(cpu_type); > > - init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer); > > - init_xml_int_attr(HELP_MIN_COUNT, event->min_count, buffer); > > + init_xml_int_attr(HELP_COUNTER_MASK, event->counter_mask, buffer, > > + MAX_BUFFER); > > + init_xml_int_attr(HELP_MIN_COUNT, event->min_count, > > + buffer, MAX_BUFFER); > > > > if (has_nested) { > > - close_xml_element(NONE, 1, buffer); > > - open_xml_element(HELP_UNIT_MASKS, 1, buffer); > > - init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, buffer); > > - close_xml_element(NONE, 1, buffer); > > + close_xml_element(NONE, 1, buffer, MAX_BUFFER); > > + open_xml_element(HELP_UNIT_MASKS, 1, buffer, MAX_BUFFER); > > + init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask, > > + buffer, MAX_BUFFER); > > + close_xml_element(NONE, 1, buffer, MAX_BUFFER); > > for (i = 0; i < event->unit->num; i++) { > > - open_xml_element(HELP_UNIT_MASK, 1, buffer); > > + open_xml_element(HELP_UNIT_MASK, 1, buffer, MAX_BUFFER); > > init_xml_int_attr(HELP_UNIT_MASK_VALUE, > > - event->unit->um[i].value, buffer); > > + event->unit->um[i].value, > > + buffer, MAX_BUFFER); > > init_xml_str_attr(HELP_UNIT_MASK_DESC, > > - event->unit->um[i].desc, buffer); > > - close_xml_element(NONE, 0, buffer); > > + event->unit->um[i].desc, > > + buffer, MAX_BUFFER); > > + close_xml_element(NONE, 0, buffer, MAX_BUFFER); > > } > > - close_xml_element(HELP_UNIT_MASKS, 0, buffer); > > + close_xml_element(HELP_UNIT_MASKS, 0, buffer, MAX_BUFFER); > > } > > - close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, buffer); > > + close_xml_element(has_nested ? HELP_EVENT : NONE, has_nested, > > + buffer, MAX_BUFFER); > > printf("%s", buffer); > > } > > > > diff --git a/libop/op_xml_out.c b/libop/op_xml_out.c > > index d779c45..7cf0abb 100644 > > --- a/libop/op_xml_out.c > > +++ b/libop/op_xml_out.c > > @@ -91,143 +91,176 @@ char const * xml_tag_name(tag_t tag) > > } > > > > > > -void open_xml_element(tag_t tag, int with_attrs, char * buffer) > > +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t max) > > { > > - char const * tag_name = xml_tag_name(tag); > > - unsigned int const max_len = strlen(tag_name) + 3; > > - char tmp_buf[MAX_BUF_LEN]; > > + char *buf; > > + int size, ret; > > > > - if (max_len >= sizeof(tmp_buf)) > > - fprintf(stderr,"Warning: open_xml_element: buffer overflow %d\n", max_len); > > + buffer[max - 1] = '\0'; > > + size = strlen(buffer); > > + buf = &buffer[size]; > > + size = max - 1 - size; > > > > - if (snprintf(tmp_buf, sizeof(tmp_buf), "<%s%s", tag_name, > > - (with_attrs ? " " : ">\n")) < 0) { > > + ret = snprintf(buf, size, "<%s%s", xml_tag_name(tag), > > + (with_attrs ? " " : ">\n")); > > + > > + if (ret < 0 || ret >= size) { > > fprintf(stderr,"open_xml_element: snprintf failed\n"); > > exit(EXIT_FAILURE); > > } > > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > > } > > > > > > -void close_xml_element(tag_t tag, int has_nested, char * buffer) > > +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t max) > > { > > - char const * tag_name = xml_tag_name(tag); > > - unsigned int const max_len = strlen(tag_name) + 3; > > - char tmp_buf[MAX_BUF_LEN]; > > + char *buf; > > + int size, ret; > > > > - if (max_len >= sizeof(tmp_buf)) > > - fprintf(stderr,"Warning: close_xml_element: buffer overflow %d\n", max_len); > > + buffer[max - 1] = '\0'; > > + size = strlen(buffer); > > + buf = &buffer[size]; > > + size = max - 1 - size; > > > > - if (tag == NONE) { > > - if (snprintf(tmp_buf, sizeof(tmp_buf), "%s\n", (has_nested ? ">" : "/>")) < 0) { > > - fprintf(stderr, "close_xml_element: snprintf failed\n"); > > - exit(EXIT_FAILURE); > > - } > > - } else { > > - if (snprintf(tmp_buf, sizeof(tmp_buf), "</%s>\n", tag_name) < 0) { > > - fprintf(stderr, "close_xml_element: snprintf failed\n"); > > - exit(EXIT_FAILURE); > > - } > > + if (tag == NONE) > > + ret = snprintf(buf, size, "%s\n", (has_nested ? ">" : "/>")); > > + else > > + ret = snprintf(buf, size, "</%s>\n", xml_tag_name(tag)); > > + > > + if (ret < 0 || ret >= size) { > > + fprintf(stderr, "close_xml_element: snprintf failed\n"); > > + exit(EXIT_FAILURE); > > } > > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > > } > > > > > > -void init_xml_int_attr(tag_t attr, int value, char * buffer) > > +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t max) > > { > > - char const * attr_name = xml_tag_name(attr); > > - char tmp_buf[MAX_BUF_LEN]; > > - unsigned int const max_len = strlen(attr_name) + 50; > > + char *buf; > > + int size, ret; > > > > - if (max_len >= sizeof(tmp_buf)) { > > - fprintf(stderr, > > - "Warning: init_xml_int_attr: buffer overflow %d\n", max_len); > > - } > > + buffer[max - 1] = '\0'; > > + size = strlen(buffer); > > + buf = &buffer[size]; > > + size = max - 1 - size; > > > > + ret = snprintf(buf, size, " %s=\"%d\"", xml_tag_name(attr), value); > > > > - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%d\"", attr_name, value) < 0) { > > + if (ret < 0 || ret >= size) { > > fprintf(stderr,"init_xml_int_attr: snprintf failed\n"); > > exit(EXIT_FAILURE); > > } > > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > > } > > > > > > -void init_xml_dbl_attr(tag_t attr, double value, char * buffer) > > +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t max) > > { > > - char const * attr_name = xml_tag_name(attr); > > - unsigned int const max_len = strlen(attr_name) + 50; > > - char tmp_buf[MAX_BUF_LEN]; > > + char *buf; > > + int size, ret; > > + > > + buffer[max - 1] = '\0'; > > + size = strlen(buffer); > > + buf = &buffer[size]; > > + size = max - 1 - size; > > > > - if (max_len >= sizeof(tmp_buf)) > > - fprintf(stderr, "Warning: init_xml_dbl_attr: buffer overflow %d\n", max_len); > > + ret = snprintf(buf, size, " %s=\"%.2f\"", xml_tag_name(attr), value); > > > > - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=\"%.2f\"", attr_name, value) < 0) { > > + if (ret < 0 || ret >= size) { > > fprintf(stderr, "init_xml_dbl_attr: snprintf failed\n"); > > exit(EXIT_FAILURE); > > } > > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > > } > > > > > > -static char * xml_quote(char const * str, char * quote_buf) > > +static void xml_quote(char const *str, char *buffer, size_t max) > > { > > - int i; > > - int pos = 0; > > - int len = strlen(str); > > + char *buf; > > + char *quote; > > + char *pos = (char*)str; > > + size_t size; > > + int ret; > > > > - > > - quote_buf[pos++] = '"'; > > + buffer[max - 1] = '\0'; > > + size = strlen(buffer); > > + buf = &buffer[size]; > > + size = max - 1 - size; > > > > - for (i = 0; i < len; i++) { > > - if (pos >= MAX_BUF_LEN - 10) { > > - fprintf(stderr,"quote_str: buffer overflow %d\n", pos); > > - exit(EXIT_FAILURE); > > - } > > + if (size < strlen(pos) + 2) > > + goto Error; > > > > - switch(str[i]) { > > + *buf = '"'; > > + buf++; > > + size--; > > + > > + while (*pos) { > > + switch(*pos) { > > case '&': > > - strncpy(quote_buf + pos, "&", 5); > > - pos += 5; > > + quote = "&"; > > break; > > case '<': > > - strncpy(quote_buf + pos, "<", 4); > > - pos += 4; > > + quote = "<"; > > break; > > case '>': > > - strncpy(quote_buf + pos, ">", 4); > > - pos += 4; > > + quote = ">"; > > break; > > case '"': > > - strncpy(quote_buf + pos, """, 6); > > - pos += 6; > > + quote = """; > > break; > > default: > > - quote_buf[pos++] = str[i]; > > - break; > > + *buf = *pos; > > + pos++; > > + buf++; > > + size--; > > + continue; > > } > > + > > + pos++; > > + ret = snprintf(buf, size, "%s", quote); > > + if (ret < 0 || ret >= (int)size) > > + goto Error; > > + buf += ret; > > + size -= ret; > > + if (size < strlen(pos)) > > + goto Error; > > } > > > > - quote_buf[pos++] = '"'; > > - quote_buf[pos++] = '\0'; > > - return quote_buf; > > + if (!size) > > + goto Error; > > + > > + *buf = '"'; > > + buf++; > > + *buf = '\0'; > > + > > + return; > > + > > +Error: > > + fprintf(stderr,"quote_str: buffer overflow\n"); > > + exit(EXIT_FAILURE); > > } > > > > > > -void init_xml_str_attr(tag_t attr, char const * str, char * buffer) > > +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t max) > > { > > - char tmp_buf[MAX_BUF_LEN]; > > - char quote_buf[MAX_BUF_LEN]; > > - char const * attr_name = xml_tag_name(attr); > > - char const * quote_str = xml_quote(str, quote_buf); > > - const unsigned int max_len = strlen(attr_name) + strlen(quote_str) + 10; > > + char *buf; > > + int size, ret; > > > > - if (max_len >= sizeof(tmp_buf)) > > - fprintf(stderr, "Warning: init_xml_str_attr: buffer overflow %d\n", max_len); > > + buffer[max - 1] = '\0'; > > + size = strlen(buffer); > > + buf = &buffer[size]; > > + size = max - 1 - size; > > > > - if (snprintf(tmp_buf, sizeof(tmp_buf), " %s=""%s""", attr_name, quote_str) < 0) { > > - fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); > > - exit(EXIT_FAILURE); > > - } > > - strncat(buffer, tmp_buf, sizeof(tmp_buf)); > > + ret = snprintf(buf, size, " %s=", xml_tag_name(attr)); > > + if (ret < 0 || ret >= size) > > + goto Error; > > + > > + buf += ret; > > + size -= ret; > > + > > + if (!size) > > + goto Error; > > + > > + xml_quote(str, buf, size); > > + return; > > +Error: > > + fprintf(stderr,"init_xml_str_attr: snprintf failed\n"); > > + exit(EXIT_FAILURE); > > } > > diff --git a/libop/op_xml_out.h b/libop/op_xml_out.h > > index 52e8d8f..ae0fea5 100644 > > --- a/libop/op_xml_out.h > > +++ b/libop/op_xml_out.h > > @@ -59,11 +59,11 @@ typedef enum { > > } tag_t; > > > > char const * xml_tag_name(tag_t tag); > > -void open_xml_element(tag_t tag, int with_attrs, char * result); > > -void close_xml_element(tag_t tag, int has_nested, char * result); > > -void init_xml_int_attr(tag_t attr, int value, char * result); > > -void init_xml_dbl_attr(tag_t attr, double value, char * result); > > -void init_xml_str_attr(tag_t attr, char const * str, char * result); > > +void open_xml_element(tag_t tag, int with_attrs, char *buffer, size_t size); > > +void close_xml_element(tag_t tag, int has_nested, char *buffer, size_t size); > > +void init_xml_int_attr(tag_t attr, int value, char *buffer, size_t size); > > +void init_xml_dbl_attr(tag_t attr, double value, char *buffer, size_t size); > > +void init_xml_str_attr(tag_t attr, char const *str, char *buffer, size_t size); > > > > #ifdef __cplusplus > > } > > diff --git a/libutil++/xml_output.cpp b/libutil++/xml_output.cpp > > index 0f0b189..4ac9a1b 100644 > > --- a/libutil++/xml_output.cpp > > +++ b/libutil++/xml_output.cpp > > @@ -16,7 +16,8 @@ > > > > using namespace std; > > > > -#define MAX_XML_BUF 1024 > > +#define MAX_XML_BUF 4096 > > +static char buf[MAX_XML_BUF]; > > > > string tag_name(tag_t tag) > > { > > @@ -29,10 +30,9 @@ string tag_name(tag_t tag) > > string open_element(tag_t tag, bool with_attrs) > > { > > ostringstream out; > > - char buf[MAX_XML_BUF]; > > > > buf[0] = '\0'; > > - open_xml_element(tag, with_attrs, buf); > > + open_xml_element(tag, with_attrs, buf, MAX_XML_BUF); > > out << buf; > > return out.str(); > > } > > @@ -41,10 +41,9 @@ string open_element(tag_t tag, bool with_attrs) > > string close_element(tag_t tag, bool has_nested) > > { > > ostringstream out; > > - char buf[MAX_XML_BUF]; > > > > buf[0] = '\0'; > > - close_xml_element(tag, has_nested, buf); > > + close_xml_element(tag, has_nested, buf, MAX_XML_BUF); > > out << buf; > > return out.str(); > > } > > @@ -53,10 +52,9 @@ string close_element(tag_t tag, bool has_nested) > > string init_attr(tag_t attr, size_t value) > > { > > ostringstream out; > > - char buf[MAX_XML_BUF]; > > > > buf[0] = '\0'; > > - init_xml_int_attr(attr, value, buf); > > + init_xml_int_attr(attr, value, buf, MAX_XML_BUF); > > out << buf; > > return out.str(); > > } > > @@ -65,10 +63,9 @@ string init_attr(tag_t attr, size_t value) > > string init_attr(tag_t attr, double value) > > { > > ostringstream out; > > - char buf[MAX_XML_BUF]; > > > > buf[0] = '\0'; > > - init_xml_dbl_attr(attr, value, buf); > > + init_xml_dbl_attr(attr, value, buf, MAX_XML_BUF); > > out << buf; > > return out.str(); > > } > > @@ -77,10 +74,9 @@ string init_attr(tag_t attr, double value) > > string init_attr(tag_t attr, string const & str) > > { > > ostringstream out; > > - char buf[MAX_XML_BUF]; > > > > buf[0] = '\0'; > > - init_xml_str_attr(attr, str.c_str(), buf); > > + init_xml_str_attr(attr, str.c_str(), buf, MAX_XML_BUF); > > out << buf; > > return out.str(); > > } > > -- Advanced Micro Devices, Inc. Operating System Research Center email: rob...@am... |