From bf4cd4adc066232a40f043e8a9ab014c291b8aba Mon Sep 17 00:00:00 2001
From: Stephen Sun <5379172+stephenxs@users.noreply.github.com>
Date: Wed, 19 Jan 2022 04:16:46 +0800
Subject: [PATCH] Fix the unsafe usage of strncpy in portsorch.cpp (#2110)
Originally, strncpy is used in the following way:
strncpy(attr.value.chardata, src_string, sizeof(attr.value.chardata));
where attr.value.chardata is a char array.
However, this is not safe in case strlen(src_string) >= sizeof(attr.value.chardata) because there will no space in attr.value.chardata to store the terminating character.
It will leave the string attr.value.chardata open, the receiver of attr cannot determine the end of the string and suffer buffer overflow.
According to SAI API definition, the actually length of SAI_HOSTIF_ATTR_NAME should be SAI_HOSTIF_NAME_SIZE - 1 which is less than sizeof(attr.value.chardata)`. So a safe way to do it should be:
strncpy(attr.value.chardata, src_string, SAI_HOSTIF_NAME_SIZE);
attr.value.chardata[SAI_HOSTIF_NAME_SIZE - 1] = '\0'
Signed-off-by: Stephen Sun <stephens@nvidia.com>
---
orchagent/portsorch.cpp | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
index ada1f4bb920b..52a35c7a3949 100755
--- a/orchagent/portsorch.cpp
+++ b/orchagent/portsorch.cpp
@@ -2172,7 +2172,12 @@ bool PortsOrch::createVlanHostIntf(Port& vl, string hostif_name)
attrs.push_back(attr);
attr.id = SAI_HOSTIF_ATTR_NAME;
- strncpy(attr.value.chardata, hostif_name.c_str(), sizeof(attr.value.chardata));
+ if (hostif_name.length() >= SAI_HOSTIF_NAME_SIZE)
+ {
+ SWSS_LOG_WARN("Host interface name %s is too long and will be truncated to %d bytes", hostif_name.c_str(), SAI_HOSTIF_NAME_SIZE - 1);
+ }
+ strncpy(attr.value.chardata, hostif_name.c_str(), SAI_HOSTIF_NAME_SIZE);
+ attr.value.chardata[SAI_HOSTIF_NAME_SIZE - 1] = '\0';
attrs.push_back(attr);
sai_status_t status = sai_hostif_api->create_hostif(&vl.m_vlan_info.host_intf_id, gSwitchId, (uint32_t)attrs.size(), attrs.data());
@@ -4186,6 +4191,11 @@ bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_int
attr.id = SAI_HOSTIF_ATTR_NAME;
strncpy((char *)&attr.value.chardata, alias.c_str(), SAI_HOSTIF_NAME_SIZE);
+ if (alias.length() >= SAI_HOSTIF_NAME_SIZE)
+ {
+ SWSS_LOG_WARN("Host interface name %s is too long and will be truncated to %d bytes", alias.c_str(), SAI_HOSTIF_NAME_SIZE - 1);
+ }
+ attr.value.chardata[SAI_HOSTIF_NAME_SIZE - 1] = '\0';
attrs.push_back(attr);
sai_status_t status = sai_hostif_api->create_hostif(&host_intfs_id, gSwitchId, (uint32_t)attrs.size(), attrs.data());