Parcourir la source

fixed an exception-unsafe bug:
dynamically allocated "getNsec3Param *" can leak when exception happens
(and it can happen in this code) before it's deleted.

Remember: code that handles a bare pointer dynamically allocated by 'new' is
almost always buggy as long as we use exceptions. let's drop that idea.

Note: in this specific case, shared_ptr is overspec because it doesn't have
to be shared. boost::scoped_ptr should be sufficient, but since we already
rely on shared_ptr, I chose to minimize dependency. (plus, NSEC3 processing
is heavy anyway, so performance overhead of shared_ptr doesn't matter much
in this context).


git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@1175 e5f2f494-b856-4b98-b285-d166d9295462

JINMEI Tatuya il y a 15 ans
Parent
commit
8fc0dcd4e1
1 fichiers modifiés avec 13 ajouts et 11 suppressions
  1. 13 11
      src/lib/auth/data_source.cc

+ 13 - 11
src/lib/auth/data_source.cc

@@ -19,6 +19,7 @@
 #include <iostream>
 #include <iostream>
 #include <vector>
 #include <vector>
 
 
+#include <boost/shared_ptr.hpp>
 #include <boost/foreach.hpp>
 #include <boost/foreach.hpp>
 
 
 #include <dns/base32.h>
 #include <dns/base32.h>
@@ -42,6 +43,8 @@ using namespace isc::dns::rdata;
 namespace isc {
 namespace isc {
 namespace auth {
 namespace auth {
 
 
+typedef boost::shared_ptr<const Nsec3Param> ConstNsec3ParamPtr;
+
 // Add a task to the query task queue to look up additional data
 // Add a task to the query task queue to look up additional data
 // (i.e., address records for the names included in NS or MX records)
 // (i.e., address records for the names included in NS or MX records)
 static void
 static void
@@ -305,9 +308,8 @@ addNSEC3(const string& hash, Query& q, const DataSrc* ds, const Name& zonename)
     return (DataSrc::SUCCESS);
     return (DataSrc::SUCCESS);
 }
 }
 
 
-static Nsec3Param*
-getNsec3Param(Query& q, const DataSrc* ds, const Name& zonename)
-{
+static ConstNsec3ParamPtr
+getNsec3Param(Query& q, const DataSrc* ds, const Name& zonename) {
     DataSrc::Result result;
     DataSrc::Result result;
     RRsetList nsec3param;
     RRsetList nsec3param;
 
 
@@ -316,12 +318,12 @@ getNsec3Param(Query& q, const DataSrc* ds, const Name& zonename)
     result = doQueryTask(ds, &zonename, q, newtask, nsec3param);
     result = doQueryTask(ds, &zonename, q, newtask, nsec3param);
     newtask.flags &= ~DataSrc::REFERRAL;
     newtask.flags &= ~DataSrc::REFERRAL;
     if (result != DataSrc::SUCCESS || newtask.flags != 0) {
     if (result != DataSrc::SUCCESS || newtask.flags != 0) {
-        return (NULL);
+        return (ConstNsec3ParamPtr());
     }
     }
 
 
     RRsetPtr rrset = nsec3param[RRType::NSEC3PARAM()];
     RRsetPtr rrset = nsec3param[RRType::NSEC3PARAM()];
     if (!rrset) {
     if (!rrset) {
-        return (NULL);
+        return (ConstNsec3ParamPtr());
     }
     }
 
 
     // XXX: currently only one NSEC3 chain per zone is supported;
     // XXX: currently only one NSEC3 chain per zone is supported;
@@ -329,28 +331,28 @@ getNsec3Param(Query& q, const DataSrc* ds, const Name& zonename)
     RdataIteratorPtr it = rrset->getRdataIterator();
     RdataIteratorPtr it = rrset->getRdataIterator();
     it->first();
     it->first();
     if (it->isLast()) {
     if (it->isLast()) {
-        return (NULL);
+        return (ConstNsec3ParamPtr());
     }
     }
 
 
     const generic::NSEC3PARAM& np =
     const generic::NSEC3PARAM& np =
             dynamic_cast<const generic::NSEC3PARAM&>(it->getCurrent());
             dynamic_cast<const generic::NSEC3PARAM&>(it->getCurrent());
-    return (new Nsec3Param(np.getHashalg(), np.getFlags(),
-                           np.getIterations(), np.getSalt()));
+    return (ConstNsec3ParamPtr(new Nsec3Param(np.getHashalg(), np.getFlags(),
+                                              np.getIterations(),
+                                              np.getSalt())));
 }
 }
 
 
 static inline DataSrc::Result
 static inline DataSrc::Result
 proveNX(Query& q, QueryTaskPtr task, const DataSrc* ds, const Name& zonename)
 proveNX(Query& q, QueryTaskPtr task, const DataSrc* ds, const Name& zonename)
 {
 {
     DataSrc::Result result;
     DataSrc::Result result;
-    Nsec3Param* nsec3 = getNsec3Param(q, ds, zonename);
-    if (nsec3) {
+    ConstNsec3ParamPtr nsec3 = getNsec3Param(q, ds, zonename);
+    if (nsec3 != NULL) {
         string node = nsec3->getHash(task->qname);
         string node = nsec3->getHash(task->qname);
         string apex = nsec3->getHash(zonename);
         string apex = nsec3->getHash(zonename);
         string wild("");
         string wild("");
         if ((task->flags & DataSrc::NAME_NOT_FOUND) != 0) {
         if ((task->flags & DataSrc::NAME_NOT_FOUND) != 0) {
             wild = nsec3->getHash(Name("*").concatenate(zonename));
             wild = nsec3->getHash(Name("*").concatenate(zonename));
         }
         }
-        delete nsec3;
 
 
         result = addNSEC3(node, q, ds, zonename);
         result = addNSEC3(node, q, ds, zonename);
         if (result != DataSrc::SUCCESS) {
         if (result != DataSrc::SUCCESS) {