Todd Greer wrote:

Hi,

While developing skip_xml_iarchive, I noticed to bugs in basic_xml_iarchive::load_end().

1. The sense of the check of the flags against no_xml_tag_checking is reversed. By default, checking of xml tag names is not done, contrary to the documentation. If no_xml_tag_checking is passed in as a flag, tag checking is enabled.

good call!!

2. If tag checking is enabled, and a tag name is encountered that is longer than the name passed in, the name string is accessed past the end of the array. In the expression "name[this->This()->gimpl->rv.object_name.size()]", the index is the size of the encountered name, and could be arbitrarily large.

The relevant portion of basic_xml_iarchive::load_end() is included here:

if(0 != (this->get_flags() & no_xml_tag_checking)){ if(0 != name[this->This()->gimpl->rv.object_name.size()] || ! std::equal( this->This()->gimpl->rv.object_name.begin(), this->This()->gimpl->rv.object_name.end(), name )

My recommended fix for (1) would be to change no_xml_tag_checking to check_xml_tags, and to change the documentation to reflect this. While the more obvious solution would be to simply fix the comparison, this would introduce a silent behavioral change. My recommended fix will fail to compile for those that have specified no_xml_tag_checking, thus alerting them to the change.

I prefer the more obvious fix as I don't want to change the docs and all the other switches are no_...

My recommended fix for (2) is to replace the excerpted code with:

if(0 != (this->get_flags() & check_xml_tags) && rv.object_name != name)

Hmmm - don't we have the same problem here? That is if the object name read is larger than "name" we could still overflow something.

The Boost FAQ recommends posting bug reports to the mailing list. Let me know if you would like for me to submit this to the bug repository.

Don't bother - we'll address it here. Robert Ramey