Re: [Boost-users] [serialization] basic_xml_iarchive tag checking bugs

From: Robert Ramey (ramey_at_[hidden])
Date: 2006-07-26 15:55:51

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