Skip to content

Commit

Permalink
Fix the unsafe usage of strncpy in portsorch.cpp (sonic-net#2110)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
stephenxs committed Jan 18, 2022
1 parent c1b4b40 commit bf4cd4a
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit bf4cd4a

Please sign in to comment.