BIND 10 #2471: use after free in DNAME processing

[BIND 10 Development]

#2471: use after free in DNAME processing
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |                       Status:  new
            Priority:  very high     |                    Milestone:  New
           Component:  b10-auth      |  Tasks
           Sensitive:  1             |                     Keywords:
         Sub-Project:  DNS           |              Defect Severity:  N/A
Estimated Difficulty:  0             |  Feature Depending on Ticket:
         Total Hours:  0             |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
 Note the "Sensitive" mark.

 There's a use-after-free bug in `Query::process`.  In the DNAME
 processing we do this:
 {{{#!cpp
             const rdata::generic::DNAME& dname(
                 dynamic_cast<const rdata::generic::DNAME&>(
                 db_context->rrset->getRdataIterator()->getCurrent()));
 }}}

 and use `dname` in the code below.  But the rdata iterator is already
 invalid at that point (the result of getCurrent() itself isn't due to
 getting the reference, but it relies on the iterator contents), and
 can cause strange behavior.  If the `RRset` is the basic one, this
 happens to be safe because it refers to the data stored in rrset,
 but in the case of in-memory data source, the rdata iterator is a
 completely temporary object, and it causes a real use-after-free
 situation.

 In the case of my personal server it crashed at this point:
 {{{#!cpp
             cname->addRdata(rdata::generic::CNAME(qname_->split(0,
                 qname_->getLabelCount() -
                 db_context->rrset->getName().getLabelCount()).
                 concatenate(dname.getDname())));
 }}}

 The fix is easy: define the rdata iterator as a separate variable, and
 keep it sufficiently long:
 {{{#!cpp
             RdataIteratorPtr rit = db_context->rrset->getRdataIterator();
             const rdata::generic::DNAME& dname(
                 dynamic_cast<const
 rdata::generic::DNAME&>(rit->getCurrent()));
 }}}

 Note that this can be a remotely exploitable problem.  I don't know
 whether we want to handle it as a "security" matter (I've marked this
 ticket as sensitive, but that's more for practice) but in any case I
 believe this should be included in the next release.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2471>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development