From 05c5c2f6b06315cb9a234dd8cdec975468848cdf Mon Sep 17 00:00:00 2001 From: Mai Bui Date: Wed, 14 Sep 2022 06:41:24 -0700 Subject: [PATCH] [swss] Replace memset functions (#2423) Signed-off-by: maipbui maibui@microsoft.com What I did Replace memset() by memset_s() Why I did it memset() is an insecure function that can cause buffer overflow. --- fpmsyncd/fpmlink.cpp | 6 +- fpmsyncd/routesync.cpp | 3 +- mclagsyncd/mclaglink.cpp | 3 +- orchagent/natorch.cpp | 190 +++++++++----------------------- tests/mock_tests/aclorch_ut.cpp | 6 +- 5 files changed, 60 insertions(+), 148 deletions(-) diff --git a/fpmsyncd/fpmlink.cpp b/fpmsyncd/fpmlink.cpp index d51b3b482aea..c26f34e062b0 100644 --- a/fpmsyncd/fpmlink.cpp +++ b/fpmsyncd/fpmlink.cpp @@ -39,7 +39,7 @@ bool FpmLink::isRawProcessing(struct nlmsghdr *h) int len; short encap_type = 0; struct rtmsg *rtm; - struct rtattr *tb[RTA_MAX + 1]; + struct rtattr *tb[RTA_MAX + 1] = {0}; rtm = (struct rtmsg *)NLMSG_DATA(h); @@ -54,7 +54,6 @@ bool FpmLink::isRawProcessing(struct nlmsghdr *h) return false; } - memset(tb, 0, sizeof(tb)); netlink_parse_rtattr(tb, RTA_MAX, RTM_RTA(rtm), len); if (!tb[RTA_MULTIPATH]) @@ -120,7 +119,7 @@ FpmLink::FpmLink(RouteSync *rsync, unsigned short port) : m_server_up(false), m_routesync(rsync) { - struct sockaddr_in addr; + struct sockaddr_in addr = {}; int true_val = 1; m_server_socket = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); @@ -141,7 +140,6 @@ FpmLink::FpmLink(RouteSync *rsync, unsigned short port) : throw system_error(errno, system_category()); } - memset (&addr, 0, sizeof (addr)); addr.sin_family = AF_INET; addr.sin_port = htons(port); addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index ab5868cdcff9..ca00d4f77da3 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -347,7 +347,7 @@ bool RouteSync::getEvpnNextHop(struct nlmsghdr *h, int received_bytes, void RouteSync::onEvpnRouteMsg(struct nlmsghdr *h, int len) { struct rtmsg *rtm; - struct rtattr *tb[RTA_MAX + 1]; + struct rtattr *tb[RTA_MAX + 1] = {0}; void *dest = NULL; char anyaddr[16] = {0}; char dstaddr[16] = {0}; @@ -360,7 +360,6 @@ void RouteSync::onEvpnRouteMsg(struct nlmsghdr *h, int len) rtm = (struct rtmsg *)NLMSG_DATA(h); /* Parse attributes and extract fields of interest. */ - memset(tb, 0, sizeof(tb)); netlink_parse_rtattr(tb, RTA_MAX, RTM_RTA(rtm), len); if (tb[RTA_DST]) diff --git a/mclagsyncd/mclaglink.cpp b/mclagsyncd/mclaglink.cpp index b09660ee56f7..ab684c54930d 100644 --- a/mclagsyncd/mclaglink.cpp +++ b/mclagsyncd/mclaglink.cpp @@ -1744,7 +1744,7 @@ MclagLink::MclagLink(Select *select, int port) : m_server_up(false), m_select(select) { - struct sockaddr_in addr; + struct sockaddr_in addr = {}; int true_val = 1; m_server_socket = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); @@ -1765,7 +1765,6 @@ MclagLink::MclagLink(Select *select, int port) : throw system_error(errno, system_category()); } - memset(&addr, 0, sizeof(addr)); addr.sin_family = AF_INET; addr.sin_port = htons((unsigned short int)port); addr.sin_addr.s_addr = htonl(MCLAG_DEFAULT_IP); diff --git a/orchagent/natorch.cpp b/orchagent/natorch.cpp index d7f124a28e65..c19f2d782386 100644 --- a/orchagent/natorch.cpp +++ b/orchagent/natorch.cpp @@ -106,8 +106,7 @@ NatOrch::NatOrch(DBConnector *appDb, DBConnector *stateDb, vectorfirst; NatEntryValue &entry = iter->second; uint32_t attr_count; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t nat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -3564,13 +3519,11 @@ bool NatOrch::getNatCounters(const NatEntry::iterator &iter) return 0; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; attr_count = 2; - memset(&nat_entry, 0, sizeof(nat_entry)); nat_entry.vr_id = gVirtualRouterId; nat_entry.switch_id = gSwitchId; @@ -3627,8 +3580,8 @@ bool NatOrch::getTwiceNatCounters(const TwiceNatEntry::iterator &iter) const TwiceNatEntryKey &key = iter->first; TwiceNatEntryValue &entry = iter->second; uint32_t attr_count; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t dbl_nat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t dbl_nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -3639,14 +3592,11 @@ bool NatOrch::getTwiceNatCounters(const TwiceNatEntry::iterator &iter) return 0; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; attr_count = 2; - memset(&dbl_nat_entry, 0, sizeof(dbl_nat_entry)); - dbl_nat_entry.vr_id = gVirtualRouterId; dbl_nat_entry.switch_id = gSwitchId; dbl_nat_entry.nat_type = SAI_NAT_TYPE_DOUBLE_NAT; @@ -3678,9 +3628,9 @@ bool NatOrch::setNatCounters(const NatEntry::iterator &iter) { const IpAddress &ipAddr = iter->first; NatEntryValue &entry = iter->second; - sai_attribute_t nat_entry_attr_packet; - sai_attribute_t nat_entry_attr_byte; - sai_nat_entry_t nat_entry; + sai_attribute_t nat_entry_attr_packet = {}; + sai_attribute_t nat_entry_attr_byte = {}; + sai_nat_entry_t nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -3690,12 +3640,8 @@ bool NatOrch::setNatCounters(const NatEntry::iterator &iter) return 0; } - memset(&nat_entry_attr_packet, 0, sizeof(nat_entry_attr_packet)); - memset(&nat_entry_attr_byte, 0, sizeof(nat_entry_attr_byte)); nat_entry_attr_byte.id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; nat_entry_attr_packet.id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; - - memset(&nat_entry, 0, sizeof(nat_entry)); nat_entry.vr_id = gVirtualRouterId; nat_entry.switch_id = gSwitchId; @@ -3762,8 +3708,8 @@ bool NatOrch::getNaptCounters(const NaptEntry::iterator &iter) NaptEntryValue &entry = iter->second; uint8_t protoType = ((naptKey.prototype == "TCP") ? IPPROTO_TCP : IPPROTO_UDP); uint32_t attr_count; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t nat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -3774,14 +3720,11 @@ bool NatOrch::getNaptCounters(const NaptEntry::iterator &iter) return 0; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; attr_count = 2; - memset(&nat_entry, 0, sizeof(nat_entry)); - nat_entry.vr_id = gVirtualRouterId; nat_entry.switch_id = gSwitchId; @@ -3848,8 +3791,8 @@ bool NatOrch::getTwiceNaptCounters(const TwiceNaptEntry::iterator &iter) TwiceNaptEntryValue &entry = iter->second; uint8_t protoType = ((key.prototype == "TCP") ? IPPROTO_TCP : IPPROTO_UDP); uint32_t attr_count; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t dbl_nat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t dbl_nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -3861,14 +3804,11 @@ bool NatOrch::getTwiceNaptCounters(const TwiceNaptEntry::iterator &iter) return 0; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; attr_count = 2; - memset(&dbl_nat_entry, 0, sizeof(dbl_nat_entry)); - dbl_nat_entry.vr_id = gVirtualRouterId; dbl_nat_entry.switch_id = gSwitchId; dbl_nat_entry.nat_type = SAI_NAT_TYPE_DOUBLE_NAT; @@ -3907,9 +3847,9 @@ bool NatOrch::setNaptCounters(const NaptEntry::iterator &iter) const NaptEntryKey &naptKey = iter->first; NaptEntryValue &entry = iter->second; uint8_t protoType = ((naptKey.prototype == "TCP") ? IPPROTO_TCP : IPPROTO_UDP); - sai_attribute_t nat_entry_attr_packet; - sai_attribute_t nat_entry_attr_byte; - sai_nat_entry_t nat_entry; + sai_attribute_t nat_entry_attr_packet = {}; + sai_attribute_t nat_entry_attr_byte = {}; + sai_nat_entry_t nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -3920,13 +3860,9 @@ bool NatOrch::setNaptCounters(const NaptEntry::iterator &iter) return 0; } - memset(&nat_entry_attr_packet, 0, sizeof(nat_entry_attr_packet)); - memset(&nat_entry_attr_byte, 0, sizeof(nat_entry_attr_byte)); nat_entry_attr_packet.id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; nat_entry_attr_byte.id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; - memset(&nat_entry, 0, sizeof(nat_entry)); - nat_entry.vr_id = gVirtualRouterId; nat_entry.switch_id = gSwitchId; @@ -4002,9 +3938,9 @@ bool NatOrch::setTwiceNatCounters(const TwiceNatEntry::iterator &iter) { const TwiceNatEntryKey &key = iter->first; TwiceNatEntryValue &entry = iter->second; - sai_attribute_t nat_entry_attr_packet; - sai_attribute_t nat_entry_attr_byte; - sai_nat_entry_t dbl_nat_entry; + sai_attribute_t nat_entry_attr_packet = {}; + sai_attribute_t nat_entry_attr_byte = {}; + sai_nat_entry_t dbl_nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -4015,13 +3951,9 @@ bool NatOrch::setTwiceNatCounters(const TwiceNatEntry::iterator &iter) return 0; } - memset(&nat_entry_attr_packet, 0, sizeof(nat_entry_attr_packet)); - memset(&nat_entry_attr_byte, 0, sizeof(nat_entry_attr_byte)); nat_entry_attr_packet.id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; nat_entry_attr_byte.id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; - memset(&dbl_nat_entry, 0, sizeof(dbl_nat_entry)); - dbl_nat_entry.vr_id = gVirtualRouterId; dbl_nat_entry.switch_id = gSwitchId; dbl_nat_entry.nat_type = SAI_NAT_TYPE_DOUBLE_NAT; @@ -4059,9 +3991,9 @@ bool NatOrch::setTwiceNaptCounters(const TwiceNaptEntry::iterator &iter) const TwiceNaptEntryKey &key = iter->first; TwiceNaptEntryValue &entry = iter->second; uint8_t protoType = ((key.prototype == "TCP") ? IPPROTO_TCP : IPPROTO_UDP); - sai_attribute_t nat_entry_attr_packet; - sai_attribute_t nat_entry_attr_byte; - sai_nat_entry_t dbl_nat_entry; + sai_attribute_t nat_entry_attr_packet = {}; + sai_attribute_t nat_entry_attr_byte = {}; + sai_nat_entry_t dbl_nat_entry = {}; sai_status_t status; uint64_t nat_translations_pkts = 0, nat_translations_bytes = 0; @@ -4072,13 +4004,9 @@ bool NatOrch::setTwiceNaptCounters(const TwiceNaptEntry::iterator &iter) return 0; } - memset(&nat_entry_attr_packet, 0, sizeof(nat_entry_attr_packet)); - memset(&nat_entry_attr_byte, 0, sizeof(nat_entry_attr_byte)); nat_entry_attr_packet.id = SAI_NAT_ENTRY_ATTR_PACKET_COUNT; nat_entry_attr_byte.id = SAI_NAT_ENTRY_ATTR_BYTE_COUNT; - memset(&dbl_nat_entry, 0, sizeof(dbl_nat_entry)); - dbl_nat_entry.vr_id = gVirtualRouterId; dbl_nat_entry.switch_id = gSwitchId; dbl_nat_entry.nat_type = SAI_NAT_TYPE_DOUBLE_NAT; @@ -4211,8 +4139,9 @@ bool NatOrch::checkIfNatEntryIsActive(const NatEntry::iterator &iter, time_t now NatEntryValue &entry = iter->second; uint32_t attr_count; IpAddress srcIp; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t snat_entry, dnat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t snat_entry = {}; + sai_nat_entry_t dnat_entry; sai_status_t status; if (entry.nat_type == "dnat") @@ -4233,7 +4162,6 @@ bool NatOrch::checkIfNatEntryIsActive(const NatEntry::iterator &iter, time_t now return 1; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_HIT_BIT; /* Get the Hit bit */ nat_entry_attr[0].value.booldata = 0; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_HIT_BIT_COR; /* clear the hit bit after returning the value */ @@ -4241,8 +4169,6 @@ bool NatOrch::checkIfNatEntryIsActive(const NatEntry::iterator &iter, time_t now attr_count = 2; - memset(&snat_entry, 0, sizeof(snat_entry)); - snat_entry.vr_id = gVirtualRouterId; snat_entry.switch_id = gSwitchId; snat_entry.nat_type = SAI_NAT_TYPE_SOURCE_NAT; @@ -4306,8 +4232,9 @@ bool NatOrch::checkIfNaptEntryIsActive(const NaptEntry::iterator &iter, time_t n uint32_t attr_count; IpAddress srcIp; uint16_t srcPort; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t snat_entry, dnat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t snat_entry = {}; + sai_nat_entry_t dnat_entry; sai_status_t status; if (entry.nat_type == "dnat") @@ -4329,7 +4256,6 @@ bool NatOrch::checkIfNaptEntryIsActive(const NaptEntry::iterator &iter, time_t n return 1; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_HIT_BIT; /* Get the Hit bit */ nat_entry_attr[0].value.booldata = 0; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_HIT_BIT_COR; /* clear the hit bit after returning the value */ @@ -4337,8 +4263,6 @@ bool NatOrch::checkIfNaptEntryIsActive(const NaptEntry::iterator &iter, time_t n attr_count = 2; - memset(&snat_entry, 0, sizeof(snat_entry)); - snat_entry.vr_id = gVirtualRouterId; snat_entry.switch_id = gSwitchId; snat_entry.nat_type = SAI_NAT_TYPE_SOURCE_NAT; @@ -4417,8 +4341,8 @@ bool NatOrch::checkIfTwiceNatEntryIsActive(const TwiceNatEntry::iterator &iter, const TwiceNatEntryKey &key = iter->first; TwiceNatEntryValue &entry = iter->second; uint32_t attr_count; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t dbl_nat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t dbl_nat_entry = {}; sai_status_t status; if (entry.entry_type == "static") @@ -4434,7 +4358,6 @@ bool NatOrch::checkIfTwiceNatEntryIsActive(const TwiceNatEntry::iterator &iter, return 0; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_HIT_BIT; /* Get the Hit bit */ nat_entry_attr[0].value.booldata = 0; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_HIT_BIT_COR; /* clear the hit bit after returning the value */ @@ -4442,8 +4365,6 @@ bool NatOrch::checkIfTwiceNatEntryIsActive(const TwiceNatEntry::iterator &iter, attr_count = 2; - memset(&dbl_nat_entry, 0, sizeof(dbl_nat_entry)); - dbl_nat_entry.vr_id = gVirtualRouterId; dbl_nat_entry.switch_id = gSwitchId; dbl_nat_entry.nat_type = SAI_NAT_TYPE_DOUBLE_NAT; @@ -4472,8 +4393,8 @@ bool NatOrch::checkIfTwiceNaptEntryIsActive(const TwiceNaptEntry::iterator &iter TwiceNaptEntryValue &entry = iter->second; uint8_t protoType = ((key.prototype == "TCP") ? IPPROTO_TCP : IPPROTO_UDP); uint32_t attr_count; - sai_attribute_t nat_entry_attr[4]; - sai_nat_entry_t dbl_nat_entry; + sai_attribute_t nat_entry_attr[4] = {}; + sai_nat_entry_t dbl_nat_entry = {}; sai_status_t status; if (entry.addedToHw == false) @@ -4489,7 +4410,6 @@ bool NatOrch::checkIfTwiceNaptEntryIsActive(const TwiceNaptEntry::iterator &iter return 1; } - memset(nat_entry_attr, 0, sizeof(nat_entry_attr)); nat_entry_attr[0].id = SAI_NAT_ENTRY_ATTR_HIT_BIT; /* Get the Hit bit */ nat_entry_attr[0].value.booldata = 0; nat_entry_attr[1].id = SAI_NAT_ENTRY_ATTR_HIT_BIT_COR; /* clear the hit bit after returning the value */ @@ -4497,8 +4417,6 @@ bool NatOrch::checkIfTwiceNaptEntryIsActive(const TwiceNaptEntry::iterator &iter attr_count = 2; - memset(&dbl_nat_entry, 0, sizeof(dbl_nat_entry)); - dbl_nat_entry.vr_id = gVirtualRouterId; dbl_nat_entry.switch_id = gSwitchId; dbl_nat_entry.nat_type = SAI_NAT_TYPE_DOUBLE_NAT; diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 9886e5d8ff17..d8fe2bbd2a5d 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -587,8 +587,7 @@ namespace aclorch_test return false; } - sai_attribute_t new_attr; - memset(&new_attr, 0, sizeof(new_attr)); + sai_attribute_t new_attr = {}; new_attr.id = attr.id; @@ -648,8 +647,7 @@ namespace aclorch_test return false; } - sai_attribute_t new_attr; - memset(&new_attr, 0, sizeof(new_attr)); + sai_attribute_t new_attr = {}; new_attr.id = attr.id;