Hello,

This patch addresses the following problem, disclosed by
recent Coverity scan:

> *** CID 1372977:  Null pointer dereferences  (FORWARD_NULL)
> /src/adaptation/icap/ModXact.cc: 1278 in
> Adaptation::Icap::ModXact::finalizeLogInfo()()
>  [ ... ]
>
>   CID 1372977:  Null pointer dereferences  (FORWARD_NULL)
> >>>     Comparing "virgin_request_" to null implies that
> >>>     "virgin_request_" might be null.
>  [ ... ]


Regards,
Eduard.
Coverity issue fix.

Adjusted Adaptation::Icap::ModXact::finalizeLogInfo(), fixing possible
"Null pointer dereference". 

=== modified file 'src/adaptation/icap/ModXact.cc'
--- src/adaptation/icap/ModXact.cc	2016-09-13 17:30:03 +0000
+++ src/adaptation/icap/ModXact.cc	2016-09-16 09:31:39 +0000
@@ -1240,69 +1240,69 @@
 Adaptation::Icap::ModXact::~ModXact()
 {
     delete bodyParser;
 }
 
 // internal cleanup
 void Adaptation::Icap::ModXact::swanSong()
 {
     debugs(93, 5, HERE << "swan sings" << status());
 
     stopWriting(false);
     stopSending(false);
 
     if (theInitiator.set()) // we have not sent the answer to the initiator
         detailError(ERR_DETAIL_ICAP_XACT_OTHER);
 
     // update adaptation history if start was called and we reserved a slot
     Adaptation::History::Pointer ah = virginRequest().adaptLogHistory();
     if (ah != NULL && adaptHistoryId >= 0)
         ah->recordXactFinish(adaptHistoryId);
 
     Adaptation::Icap::Xaction::swanSong();
 }
 
 void prepareLogWithRequestDetails(HttpRequest *, AccessLogEntry::Pointer &);
 
 void Adaptation::Icap::ModXact::finalizeLogInfo()
 {
     HttpRequest *adapted_request_ = nullptr;
     HttpReply *adapted_reply_ = nullptr;
-    HttpRequest *virgin_request_ = (virgin.cause ? virgin.cause : dynamic_cast<HttpRequest*>(virgin.header));
+    HttpRequest *virgin_request_ = const_cast<HttpRequest*>(&virginRequest());
     if (!(adapted_request_ = dynamic_cast<HttpRequest*>(adapted.header))) {
         // if the request was not adapted, use virgin request to simplify
         // the code further below
         adapted_request_ = virgin_request_;
         adapted_reply_ = dynamic_cast<HttpReply*>(adapted.header);
     }
 
-    Adaptation::Icap::History::Pointer h = (virgin_request_ ? virgin_request_->icapHistory() : NULL);
+    Adaptation::Icap::History::Pointer h = virgin_request_->icapHistory();
     Must(h != NULL); // ICAPXaction::maybeLog calls only if there is a log
     al.icp.opcode = ICP_INVALID;
     al.url = h->log_uri.termedBuf();
     const Adaptation::Icap::ServiceRep  &s = service();
     al.icap.reqMethod = s.cfg().method;
 
     al.cache.caddr = virgin_request_->client_addr;
 
     al.request = virgin_request_;
     HTTPMSGLOCK(al.request);
     al.adapted_request = adapted_request_;
     HTTPMSGLOCK(al.adapted_request);
 
     if (adapted_reply_) {
         al.reply = adapted_reply_;
         HTTPMSGLOCK(al.reply);
     } else
         al.reply = NULL;
 
     if (h->rfc931.size())
         al.cache.rfc931 = h->rfc931.termedBuf();
 
 #if USE_OPENSSL
     if (h->ssluser.size())
         al.cache.ssluser = h->ssluser.termedBuf();
 #endif
     al.cache.code = h->logType;
 
     const HttpMsg *virgin_msg = dynamic_cast<HttpReply*>(virgin.header);
     if (!virgin_msg)

_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to