matthew/sonic-buildimage
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

[dhcp6relay] a couple memory access protections (#9851)

Browse Source 
Why I did it
the strcpy and buffer allocation is not safe, it corrupts 1 byte on the stack. Depending on the memory layout, it may or may not cause issue immediately.
message type is not validated before updating the counter. Which could cause segment fault.

How I did it
Remove the unsafe strcpy, use config->interface.c_str() instead.
Check message type before updating counters.

How to verify it
The issue (1) caused segment fault on a specific platform. The fix was validated there. Issue (2) was precautionary. Added log in case it triggers.
This commit is contained in:
Ying Xie 2022-01-26 09:58:52 -08:00 committed by Judy Joseph
parent 4df0aa348c
commit 53a8d99baa
2 changed files with 8 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/dhcp6relay/src/configInterface.cpp
View File

@ -119,6 +119,7 @@ void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries,
relay_config intf;
intf.is_option_79 = true;
intf.interface = vlan;
intf.db = nullptr;
for (auto &fieldValue: fieldValues) {
std::string f = fvField(fieldValue);
std::string v = fvValue(fieldValue);

15
src/dhcp6relay/src/relay.cpp
View File

@ -226,9 +226,11 @@ void send_udp(int sock, uint8_t *buffer, struct sockaddr_in6 target, uint32_t n,
std::string counterVlan = counter_table;
if(sendto(sock, buffer, n, 0, (const struct sockaddr *)&target, sizeof(target)) == -1)
syslog(LOG_ERR, "sendto: Failed to send to target address\n");
else {
else if (counterMap.find(msg_type) != counterMap.end()) {
counters[msg_type]++;
update_counter(config->db, counterVlan.append(config->interface), msg_type);
} else {
syslog(LOG_WARNING, "unexpected message type %d(0x%x)\n", msg_type, msg_type);
}
}
@ -518,10 +520,9 @@ void relay_relay_forw(int sock, const uint8_t *msg, int32_t len, const ip6_hdr *
*
* @return none
*/
void relay_relay_reply(int sock, const uint8_t *msg, int32_t len, relay_config *configs) {
void relay_relay_reply(int sock, const uint8_t *msg, int32_t len, relay_config *config) {
static uint8_t buffer[4096];
uint8_t type = 0;
char ifname[configs->interface.size()];
struct sockaddr_in6 target_addr;
auto current_buffer_position = buffer;
auto current_position = msg;
@ -546,14 +547,13 @@ void relay_relay_forw(int sock, const uint8_t *msg, int32_t len, const ip6_hdr *
}
}
strcpy(ifname, configs->interface.c_str());
memcpy(&target_addr.sin6_addr, &dhcp_relay_header->peer_address, sizeof(struct in6_addr));
target_addr.sin6_family = AF_INET6;
target_addr.sin6_flowinfo = 0;
target_addr.sin6_port = htons(CLIENT_PORT);
target_addr.sin6_scope_id = if_nametoindex(ifname);
target_addr.sin6_scope_id = if_nametoindex(config->interface.c_str());
send_udp(sock, buffer, target_addr, current_buffer_position - buffer, configs, type);
send_udp(sock, buffer, target_addr, current_buffer_position - buffer, config, type);
}
@ -758,8 +758,7 @@ void loop_relay(std::vector<relay_config> *vlans, swss::DBConnector *db) {
int filter = 0;
int local_sock = 0;
int server_sock = 0;
const char *ifname = config->interface.c_str();
int index = if_nametoindex(ifname);
int index = if_nametoindex(config->interface.c_str());
config->db = db;
std::string counterVlan = counter_table;