From: Remi Gacogne Date: Mon, 27 Jun 2016 15:50:16 +0000 (+0200) Subject: auth: Minor fixes to dnspcap{,2protobuf}, reported by coverity X-Git-Tag: auth-4.0.0-rc1~11^2~1 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d77299689d39cb89c2f4fd8be9f52bc1819db728;p=pdns auth: Minor fixes to dnspcap{,2protobuf}, reported by coverity --- diff --git a/pdns/dnspcap.cc b/pdns/dnspcap.cc index c9a4bd41a..c8c553afd 100644 --- a/pdns/dnspcap.cc +++ b/pdns/dnspcap.cc @@ -172,6 +172,10 @@ PcapPacketWriter::PcapPacketWriter(const string& fname) : d_fname(fname) void PcapPacketWriter::write() { + if (!d_ppr) { + return; + } + if(d_first) { fwrite(&d_ppr->d_pfh, 1, sizeof(d_ppr->d_pfh), d_fp); d_first=false; diff --git a/pdns/dnspcap.hh b/pdns/dnspcap.hh index 99365720e..6f62ef9a0 100644 --- a/pdns/dnspcap.hh +++ b/pdns/dnspcap.hh @@ -40,14 +40,14 @@ struct pdns_pcap_file_header { struct pdns_timeval { - uint32_t tv_sec; - uint32_t tv_usec; + uint32_t tv_sec{0}; + uint32_t tv_usec{0}; }; struct pdns_pcap_pkthdr { struct pdns_timeval ts; /* time stamp */ - uint32_t caplen; /* length of portion present */ - uint32_t len; /* length this packet (off wire) */ + uint32_t caplen{0}; /* length of portion present */ + uint32_t len{0}; /* length this packet (off wire) */ }; struct pdns_lcc_header { @@ -86,14 +86,14 @@ public: ComboAddress getSource() const; ComboAddress getDest() const; - struct pdns_lcc_header* d_lcc; - struct ether_header* d_ether; - struct ip *d_ip; - struct ip6_hdr *d_ip6; - const struct tcphdr *d_tcp; - const struct udphdr *d_udp; - const uint8_t* d_payload; - unsigned int d_len; + struct pdns_lcc_header* d_lcc{nullptr}; + struct ether_header* d_ether{nullptr}; + struct ip *d_ip{nullptr}; + struct ip6_hdr *d_ip6{nullptr}; + const struct tcphdr *d_tcp{nullptr}; + const struct udphdr *d_udp{nullptr}; + const uint8_t* d_payload{nullptr}; + unsigned int d_len{0}; struct pdns_pcap_pkthdr d_pheader; pdns_pcap_file_header d_pfh; @@ -117,7 +117,7 @@ public: private: string d_fname; - const PcapPacketReader* d_ppr; + const PcapPacketReader* d_ppr{nullptr}; FILE *d_fp; bool d_first{true}; diff --git a/pdns/dnspcap2protobuf.cc b/pdns/dnspcap2protobuf.cc index 1f33ff178..d7d237266 100644 --- a/pdns/dnspcap2protobuf.cc +++ b/pdns/dnspcap2protobuf.cc @@ -74,10 +74,14 @@ try } } } -catch(std::exception& e) +catch(const std::exception& e) { cerr<<"Error parsing response records: "< ids; boost::uuids::random_generator uuidGenerator; - while (pr.getUDPPacket()) { - const dnsheader* dh=(dnsheader*)pr.d_payload; - if (!dh->qdcount) - continue; - - if (pr.d_len < sizeof(dnsheader)) - continue; - - if(!dh->rd) - continue; - - uint16_t qtype, qclass; - DNSName qname; - try { - qname=DNSName((const char*)pr.d_payload, pr.d_len, sizeof(dnsheader), false, &qtype, &qclass); - } - catch(const std::exception& e) { - cerr<<"Error while parsing qname: "<qdcount) + continue; + + if (pr.d_len < sizeof(dnsheader)) + continue; + + if(!dh->rd) + continue; + + uint16_t qtype, qclass; + DNSName qname; + try { + qname=DNSName((const char*)pr.d_payload, pr.d_len, sizeof(dnsheader), false, &qtype, &qclass); + } + catch(const std::exception& e) { + cerr<<"Error while parsing qname: "<id)); - message.set_type(dh->qr ? PBDNSMessage_Type_DNSResponseType : PBDNSMessage_Type_DNSQueryType); - const ComboAddress requestor = dh->qr ? pr.getDest() : pr.getSource(); - const ComboAddress responder = dh->qr ? pr.getSource() : pr.getDest(); - - *((char*)&requestor.sin4.sin_addr.s_addr)|=ind; - *((char*)&responder.sin4.sin_addr.s_addr)|=ind; - message.set_socketfamily(requestor.sin4.sin_family == AF_INET ? PBDNSMessage_SocketFamily_INET : PBDNSMessage_SocketFamily_INET6); - // we handle UDP packets only for now - message.set_socketprotocol(PBDNSMessage_SocketProtocol_UDP); - if (requestor.sin4.sin_family == AF_INET) { - message.set_from(&requestor.sin4.sin_addr.s_addr, sizeof(requestor.sin4.sin_addr.s_addr)); - } - else if (requestor.sin4.sin_family == AF_INET6) { - message.set_from(&requestor.sin6.sin6_addr.s6_addr, sizeof(requestor.sin6.sin6_addr.s6_addr)); - } - if (responder.sin4.sin_family == AF_INET) { - message.set_to(&responder.sin4.sin_addr.s_addr, sizeof(responder.sin4.sin_addr.s_addr)); - } - else if (responder.sin4.sin_family == AF_INET6) { - message.set_to(&responder.sin6.sin6_addr.s6_addr, sizeof(responder.sin6.sin6_addr.s6_addr)); - } - message.set_inbytes(pr.d_len); + PBDNSMessage message; + message.set_timesec(pr.d_pheader.ts.tv_sec); + message.set_timeusec(pr.d_pheader.ts.tv_usec); + message.set_id(ntohs(dh->id)); + message.set_type(dh->qr ? PBDNSMessage_Type_DNSResponseType : PBDNSMessage_Type_DNSQueryType); + const ComboAddress requestor = dh->qr ? pr.getDest() : pr.getSource(); + const ComboAddress responder = dh->qr ? pr.getSource() : pr.getDest(); + + *((char*)&requestor.sin4.sin_addr.s_addr)|=ind; + *((char*)&responder.sin4.sin_addr.s_addr)|=ind; + message.set_socketfamily(requestor.sin4.sin_family == AF_INET ? PBDNSMessage_SocketFamily_INET : PBDNSMessage_SocketFamily_INET6); + // we handle UDP packets only for now + message.set_socketprotocol(PBDNSMessage_SocketProtocol_UDP); + if (requestor.sin4.sin_family == AF_INET) { + message.set_from(&requestor.sin4.sin_addr.s_addr, sizeof(requestor.sin4.sin_addr.s_addr)); + } + else if (requestor.sin4.sin_family == AF_INET6) { + message.set_from(&requestor.sin6.sin6_addr.s6_addr, sizeof(requestor.sin6.sin6_addr.s6_addr)); + } + if (responder.sin4.sin_family == AF_INET) { + message.set_to(&responder.sin4.sin_addr.s_addr, sizeof(responder.sin4.sin_addr.s_addr)); + } + else if (responder.sin4.sin_family == AF_INET6) { + message.set_to(&responder.sin6.sin6_addr.s6_addr, sizeof(responder.sin6.sin6_addr.s6_addr)); + } + message.set_inbytes(pr.d_len); - PBDNSMessage_DNSQuestion* question = message.mutable_question(); - PBDNSMessage_DNSResponse* response = message.mutable_response(); + PBDNSMessage_DNSQuestion* question = message.mutable_question(); + PBDNSMessage_DNSResponse* response = message.mutable_response(); - if (!dh->qr) { - boost::uuids::uuid uniqueId = uuidGenerator(); - ids[dh->id] = uniqueId; - std::string* messageId = message.mutable_messageid(); - messageId->resize(uniqueId.size()); - std::copy(uniqueId.begin(), uniqueId.end(), messageId->begin()); - } - else { - const auto& it = ids.find(dh->id); - if (it != ids.end()) { + if (!dh->qr) { + boost::uuids::uuid uniqueId = uuidGenerator(); + ids[dh->id] = uniqueId; std::string* messageId = message.mutable_messageid(); - messageId->resize(it->second.size()); - std::copy(it->second.begin(), it->second.end(), messageId->begin()); + messageId->resize(uniqueId.size()); + std::copy(uniqueId.begin(), uniqueId.end(), messageId->begin()); + } + else { + const auto& it = ids.find(dh->id); + if (it != ids.end()) { + std::string* messageId = message.mutable_messageid(); + messageId->resize(it->second.size()); + std::copy(it->second.begin(), it->second.end(), messageId->begin()); + } + + response->set_rcode(dh->rcode); + addRRs((const char*) dh, pr.d_len, response); } - response->set_rcode(dh->rcode); - addRRs((const char*) dh, pr.d_len, response); - } - - question->set_qname(qname.toString()); - question->set_qtype(qtype); - question->set_qclass(qclass); + question->set_qname(qname.toString()); + question->set_qtype(qtype); + question->set_qclass(qclass); - std::string str; - //cerr<