From df105a79e4549a5bfa7a8244c31d9b3336cc7c5e Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Tue, 4 Jun 2024 13:26:04 +0300 Subject: [PATCH] Refactor of IP utils to use exceptions on invalid input. (#1426) * Changed IpUtils to raise exceptions on invalid input instead of returning nullptr. * Added 'try' methods for extraction of IP address from socket address. * Updated calls to sockaddr2* to try_sockaddr2* to keep current functionality. * Lint * Added overload of sockaddr2string that returns std::string. * Added debug logs about failed extractions. * Lint * Removed 'struct' keywords from function parameters. * Changed C-style cast to Cpp cast * Changed reference to pointer as most usages use pointer. * C cast -> Cpp cast * Added length parameter to sockaddr2string to protect against buffer overflow. * Lint * Removed struct keyword from parameter. * Changed inet_ntop to be passed the actual length of the buffer. Added runtime exception if inet_ntop fails for some reason. * Removed unused function * Changed switch to if statement. * Fixed accidentally removing wrong function. * Update Common++/header/IpUtils.h Removed unused include. --- Common++/header/IpUtils.h | 26 +++++++-- Common++/src/IpUtils.cpp | 86 +++++++++++++++++++++++------ Pcap++/src/PcapLiveDevice.cpp | 23 ++++---- Pcap++/src/PcapLiveDeviceList.cpp | 18 +++--- Pcap++/src/PcapRemoteDeviceList.cpp | 17 +++--- 5 files changed, 121 insertions(+), 49 deletions(-) diff --git a/Common++/header/IpUtils.h b/Common++/header/IpUtils.h index cd35ed58c9..6ad4d83146 100644 --- a/Common++/header/IpUtils.h +++ b/Common++/header/IpUtils.h @@ -62,22 +62,40 @@ namespace pcpp * Extract IPv4 address from sockaddr * @param[in] sa - input sockaddr * @return Address in in_addr format + * @throws std::invalid_argument Sockaddr family is not AF_INET or sockaddr is nullptr. */ - in_addr* sockaddr2in_addr(struct sockaddr *sa); + in_addr* sockaddr2in_addr(sockaddr* sa); + + /** + * Attempt to extract IPv4 address from sockaddr + * @param[in] sa - input sockaddr + * @return Pointer to address in in_addr format or nullptr if extraction fails. + */ + in_addr* try_sockaddr2in_addr(sockaddr* sa); /** * Extract IPv6 address from sockaddr * @param[in] sa - input sockaddr * @return Address in in6_addr format + * @throws std::invalid_argument Sockaddr family is not AF_INET6 or sockaddr is nullptr. + */ + in6_addr* sockaddr2in6_addr(sockaddr* sa); + + /** + * Attempt to extract IPv6 address from sockaddr + * @param[in] sa - input sockaddr + * @return Pointer to address in in6_addr format or nullptr if extraction fails. */ - in6_addr* sockaddr2in6_addr(struct sockaddr *sa); + in6_addr* try_sockaddr2in6_addr(sockaddr* sa); /** * Converts a sockaddr format address to its string representation * @param[in] sa Address in sockaddr format - * @param[out] resultString String representation of the address + * @param[out] resultString String representation of the address + * @param[in] resultBufLen Length of the result buffer. + * @throws std::invalid_argument Sockaddr family is not AF_INET or AF_INET6, sockaddr is nullptr or the result str buffer is insufficient. */ - void sockaddr2string(struct sockaddr *sa, char* resultString); + void sockaddr2string(sockaddr const* sa, char* resultString, size_t resultBufLen); /** * Convert a in_addr format address to 32bit representation diff --git a/Common++/src/IpUtils.cpp b/Common++/src/IpUtils.cpp index c78a1807f5..04337eae30 100644 --- a/Common++/src/IpUtils.cpp +++ b/Common++/src/IpUtils.cpp @@ -4,6 +4,7 @@ #include "Logger.h" #include #include +#include #ifndef NS_INADDRSZ #define NS_INADDRSZ 4 #endif @@ -18,36 +19,87 @@ namespace pcpp { namespace internal { - in_addr* sockaddr2in_addr(struct sockaddr* sa) + in_addr* sockaddr2in_addr(sockaddr* sa) { if (sa == nullptr) + throw std::invalid_argument("sockaddr is nullptr"); + + if (sa->sa_family != AF_INET) + throw std::invalid_argument("sockaddr family is not AF_INET."); + + return &(reinterpret_cast(sa)->sin_addr); + } + + in_addr* try_sockaddr2in_addr(sockaddr* sa) + { + try + { + return sockaddr2in_addr(sa); + } + catch (const std::invalid_argument& e) + { + PCPP_LOG_DEBUG("Extraction failed: " << e.what() << " Returning nullptr."); return nullptr; - if (sa->sa_family == AF_INET) - return &(((struct sockaddr_in*)sa)->sin_addr); - PCPP_LOG_DEBUG("sockaddr family is not AF_INET. Returning NULL"); - return nullptr; + } } - in6_addr* sockaddr2in6_addr(struct sockaddr* sa) + in6_addr* sockaddr2in6_addr(sockaddr* sa) { - if (sa->sa_family == AF_INET6) - return &(((struct sockaddr_in6*)sa)->sin6_addr); - PCPP_LOG_DEBUG("sockaddr family is not AF_INET6. Returning NULL"); - return nullptr; + if (sa == nullptr) + throw std::invalid_argument("sockaddr is nullptr"); + + if (sa->sa_family != AF_INET6) + throw std::invalid_argument("sockaddr family is not AF_INET6."); + + return &(reinterpret_cast(sa)->sin6_addr); } - void sockaddr2string(struct sockaddr* sa, char* resultString) + in6_addr* try_sockaddr2in6_addr(sockaddr* sa) { - in_addr* ipv4Addr = sockaddr2in_addr(sa); - if (ipv4Addr != nullptr) + try + { + return sockaddr2in6_addr(sa); + } + catch (const std::invalid_argument& e) + { + PCPP_LOG_DEBUG("Extraction failed: " << e.what() << " Returning nullptr."); + return nullptr; + } + } + + void sockaddr2string(sockaddr const* sa, char* resultString, size_t resultBufLen) + { + if (sa == nullptr) + throw std::invalid_argument("sockaddr is nullptr"); + + switch (sa->sa_family) + { + case AF_INET: { PCPP_LOG_DEBUG("IPv4 packet address"); - inet_ntop(AF_INET, &(((sockaddr_in*)sa)->sin_addr), resultString, INET_ADDRSTRLEN); + if (resultBufLen < INET_ADDRSTRLEN) + throw std::invalid_argument("Insufficient buffer"); + + if (inet_ntop(AF_INET, &(reinterpret_cast(sa)->sin_addr), resultString, resultBufLen) == nullptr) + { + throw std::runtime_error("Unknown error during conversion"); + } + break; } - else + case AF_INET6: { - PCPP_LOG_DEBUG("Not IPv4 packet address. Assuming IPv6 packet"); - inet_ntop(AF_INET6, &(((sockaddr_in6*)sa)->sin6_addr), resultString, INET6_ADDRSTRLEN); + PCPP_LOG_DEBUG("IPv6 packet address"); + if (resultBufLen < INET6_ADDRSTRLEN) + throw std::invalid_argument("Insufficient buffer"); + + if(inet_ntop(AF_INET6, &(reinterpret_cast(sa)->sin6_addr), resultString, resultBufLen) == nullptr) + { + throw std::runtime_error("Unknown error during conversion"); + } + break; + } + default: + throw std::invalid_argument("Unsupported sockaddr family. Family is not AF_INET or AF_INET6."); } } diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 7a55a0a2bf..dcc32f0b74 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #if defined(_WIN32) // The definition of BPF_MAJOR_VERSION is required to support Npcap. In Npcap there are // compilation errors due to struct redefinition when including both Packet32.h and pcap.h @@ -93,9 +94,9 @@ PcapLiveDevice::PcapLiveDevice(pcap_if_t* pInterface, bool calculateMTU, bool ca pInterface->addresses = pInterface->addresses->next; if (Logger::getInstance().isDebugEnabled(PcapLogModuleLiveDevice) && pInterface->addresses != nullptr && pInterface->addresses->addr != nullptr) { - char addrAsString[INET6_ADDRSTRLEN]; - internal::sockaddr2string(pInterface->addresses->addr, addrAsString); - PCPP_LOG_DEBUG(" " << addrAsString); + std::array addrAsString; + internal::sockaddr2string(pInterface->addresses->addr, addrAsString.data(), addrAsString.size()); + PCPP_LOG_DEBUG(" " << addrAsString.data()); } } @@ -1094,12 +1095,12 @@ IPv4Address PcapLiveDevice::getIPv4Address() const { if (Logger::getInstance().isDebugEnabled(PcapLogModuleLiveDevice) && addrIter.addr != nullptr) { - char addrAsString[INET6_ADDRSTRLEN]; - internal::sockaddr2string(addrIter.addr, addrAsString); - PCPP_LOG_DEBUG("Searching address " << addrAsString); + std::array addrAsString; + internal::sockaddr2string(addrIter.addr, addrAsString.data(), addrAsString.size()); + PCPP_LOG_DEBUG("Searching address " << addrAsString.data()); } - in_addr* currAddr = internal::sockaddr2in_addr(addrIter.addr); + in_addr* currAddr = internal::try_sockaddr2in_addr(addrIter.addr); if (currAddr == nullptr) { PCPP_LOG_DEBUG("Address is NULL"); @@ -1125,11 +1126,11 @@ IPv6Address PcapLiveDevice::getIPv6Address() const { if (Logger::getInstance().isDebugEnabled(PcapLogModuleLiveDevice) && addrIter.addr != nullptr) { - char addrAsString[INET6_ADDRSTRLEN]; - internal::sockaddr2string(addrIter.addr, addrAsString); - PCPP_LOG_DEBUG("Searching address " << addrAsString); + std::array addrAsString; + internal::sockaddr2string(addrIter.addr, addrAsString.data(), addrAsString.size()); + PCPP_LOG_DEBUG("Searching address " << addrAsString.data()); } - in6_addr *currAddr = internal::sockaddr2in6_addr(addrIter.addr); + in6_addr *currAddr = internal::try_sockaddr2in6_addr(addrIter.addr); if (currAddr == nullptr) { PCPP_LOG_DEBUG("Address is NULL"); diff --git a/Pcap++/src/PcapLiveDeviceList.cpp b/Pcap++/src/PcapLiveDeviceList.cpp index b1d238a3cc..47f179f479 100644 --- a/Pcap++/src/PcapLiveDeviceList.cpp +++ b/Pcap++/src/PcapLiveDeviceList.cpp @@ -236,7 +236,7 @@ void PcapLiveDeviceList::setDnsServers() sockaddr* saddr = (sockaddr*)&_res.nsaddr_list[i]; if (saddr == nullptr) continue; - in_addr* inaddr = internal::sockaddr2in_addr(saddr); + in_addr* inaddr = internal::try_sockaddr2in_addr(saddr); if (inaddr == nullptr) continue; @@ -275,12 +275,12 @@ PcapLiveDevice* PcapLiveDeviceList::getPcapLiveDeviceByIp(const IPv4Address& ipA { if (Logger::getInstance().isDebugEnabled(PcapLogModuleLiveDevice) && addrIter.addr != nullptr) { - char addrAsString[INET6_ADDRSTRLEN]; - internal::sockaddr2string(addrIter.addr, addrAsString); - PCPP_LOG_DEBUG("Searching address " << addrAsString); + std::array addrAsString; + internal::sockaddr2string(addrIter.addr, addrAsString.data(), addrAsString.size()); + PCPP_LOG_DEBUG("Searching address " << addrAsString.data()); } - in_addr* currAddr = internal::sockaddr2in_addr(addrIter.addr); + in_addr* currAddr = internal::try_sockaddr2in_addr(addrIter.addr); if (currAddr == nullptr) { PCPP_LOG_DEBUG("Address is nullptr"); @@ -308,12 +308,12 @@ PcapLiveDevice* PcapLiveDeviceList::getPcapLiveDeviceByIp(const IPv6Address& ip6 { if (Logger::getInstance().isDebugEnabled(PcapLogModuleLiveDevice) && addrIter.addr != nullptr) { - char addrAsString[INET6_ADDRSTRLEN]; - internal::sockaddr2string(addrIter.addr, addrAsString); - PCPP_LOG_DEBUG("Searching address " << addrAsString); + std::array addrAsString; + internal::sockaddr2string(addrIter.addr, addrAsString.data(), addrAsString.size()); + PCPP_LOG_DEBUG("Searching address " << addrAsString.data()); } - in6_addr* currAddr = internal::sockaddr2in6_addr(addrIter.addr); + in6_addr* currAddr = internal::try_sockaddr2in6_addr(addrIter.addr); if (currAddr == nullptr) { PCPP_LOG_DEBUG("Address is nullptr"); diff --git a/Pcap++/src/PcapRemoteDeviceList.cpp b/Pcap++/src/PcapRemoteDeviceList.cpp index b4797e97c2..af107fee91 100644 --- a/Pcap++/src/PcapRemoteDeviceList.cpp +++ b/Pcap++/src/PcapRemoteDeviceList.cpp @@ -7,6 +7,7 @@ #include "IpUtils.h" #include "IpAddressUtils.h" #include "pcap.h" +#include #include namespace pcpp @@ -108,12 +109,12 @@ PcapRemoteDevice* PcapRemoteDeviceList::getRemoteDeviceByIP(const IPv4Address& i { if (Logger::getInstance().isDebugEnabled(PcapLogModuleRemoteDevice) && addrIter.addr != NULL) { - char addrAsString[INET6_ADDRSTRLEN]; - internal::sockaddr2string(addrIter.addr, addrAsString); - PCPP_LOG_DEBUG("Searching address " << addrAsString); + std::array addrAsString; + internal::sockaddr2string(addrIter.addr, addrAsString.data(), addrAsString.size()); + PCPP_LOG_DEBUG("Searching address " << addrAsString.data()); } - in_addr* currAddr = internal::sockaddr2in_addr(addrIter.addr); + in_addr* currAddr = internal::try_sockaddr2in_addr(addrIter.addr); if (currAddr == NULL) { PCPP_LOG_DEBUG("Address is NULL"); @@ -142,12 +143,12 @@ PcapRemoteDevice* PcapRemoteDeviceList::getRemoteDeviceByIP(const IPv6Address& i { if (Logger::getInstance().isDebugEnabled(PcapLogModuleRemoteDevice) && addrIter.addr != NULL) { - char addrAsString[INET6_ADDRSTRLEN]; - internal::sockaddr2string(addrIter.addr, addrAsString); - PCPP_LOG_DEBUG("Searching address " << addrAsString); + std::array addrAsString; + internal::sockaddr2string(addrIter.addr, addrAsString.data(), addrAsString.size()); + PCPP_LOG_DEBUG("Searching address " << addrAsString.data()); } - in6_addr* currAddr = internal::sockaddr2in6_addr(addrIter.addr); + in6_addr* currAddr = internal::try_sockaddr2in6_addr(addrIter.addr); if (currAddr == NULL) { PCPP_LOG_DEBUG("Address is NULL");