RDKB-65853: Fixing Vulnerability issue#1265
Open
sshah500 wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets vulnerability-related memory issues in the webconfig translation and RBUS handler paths by adding explicit cleanup on failure, and by ensuring encoded webconfig buffers are properly freed.
Changes:
- Add
goto cleanuperror paths in multiple OVSDB-to-RDK translators to free partially-built maps/allocations on failure. - Switch
apply_ignite_config()to allocatewebconfig_subdoc_data_ton the heap and free it after encoding. - Fix a leak in
set_force_vap_apply()by callingwebconfig_data_free()on the success path before freeing the wrapper struct.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| source/webconfig/wifi_ovsdb_translator.c | Adds centralized cleanup paths to destroy partially populated hash maps and free in-loop allocations on translation failures. |
| source/core/wifi_ctrl_rbus_handlers.c | Moves ignite webconfig subdoc data to heap allocation and ensures encoded buffers are freed on the force-apply success path. |
Comments suppressed due to low confidence (1)
source/core/wifi_ctrl_rbus_handlers.c:2878
datawas changed to a heap pointer, but this memcpy still usesdata.u...(struct access) instead ofdata->u..., which will not compile and won’t copy into the allocated buffer.
memcpy(&data.u.decoded.ignite_config, &g_apply_ignite_config.config,
num_of_radios * sizeof(ignite_config_t));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (webconfig_encode(&ctrl->webconfig, data, webconfig_subdoc_type_ignite) | ||
| == webconfig_error_none) { | ||
| wifi_util_info_print(WIFI_CTRL, "%s:%d webconfig_encode success\n", __FUNCTION__, __LINE__); | ||
| str = (char *)data.u.encoded.raw; |
Comment on lines
4714
to
+4716
| memcpy(stat_config_entry, &temp_stat_config_entry, sizeof(stats_config_t)); | ||
| hash_map_put(data->u.decoded.stats_config_map, strdup(temp_stat_config_entry.stats_cfg_id), stat_config_entry); | ||
| stat_config_entry = NULL; |
Comment on lines
4852
to
+4855
| memset(steer_config_entry, 0, sizeof(steering_config_t)); | ||
| memcpy(steer_config_entry, &temp_steer_config, sizeof(steering_config_t)); | ||
| hash_map_put(data->u.decoded.steering_config_map, strdup(temp_steer_config.steering_cfg_id), steer_config_entry); | ||
| steer_config_entry = NULL; |
Comment on lines
5204
to
+5207
| memset(steering_client_entry, 0, sizeof(band_steering_clients_t)); | ||
| memcpy(steering_client_entry, &temp_steering_client, sizeof(band_steering_clients_t)); | ||
| hash_map_put(data->u.decoded.steering_client_map, strdup(temp_steering_client.steering_client_id), steering_client_entry); | ||
| steering_client_entry = NULL; |
Comment on lines
5309
to
+5312
| memset(vif_neighbor_entry, 0, sizeof(vif_neighbors_t)); | ||
| memcpy(vif_neighbor_entry, &temp_vif_neighbor, sizeof(vif_neighbors_t)); | ||
| hash_map_put(data->u.decoded.vif_neighbors_map, strdup(temp_vif_neighbor.neighbor_id), vif_neighbor_entry); | ||
| vif_neighbor_entry = NULL; |
sshah500
force-pushed
the
test_coverity_bkp
branch
from
July 14, 2026 12:03
30906f7 to
edb544f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
source/core/wifi_ctrl_rbus_handlers.c:2878
datawas changed to a pointer, but thismemcpystill usesdata.u...(stack-style). This will not compile and also won’t copy into the allocated struct.
// Copy pending config to data
memcpy(&data.u.decoded.ignite_config, &g_apply_ignite_config.config,
num_of_radios * sizeof(ignite_config_t));
Comment on lines
4713
to
+4716
| memset(stat_config_entry, 0, sizeof(stats_config_t)); | ||
| memcpy(stat_config_entry, &temp_stat_config_entry, sizeof(stats_config_t)); | ||
| hash_map_put(data->u.decoded.stats_config_map, strdup(temp_stat_config_entry.stats_cfg_id), stat_config_entry); | ||
| stat_config_entry = NULL; |
Comment on lines
4852
to
+4855
| memset(steer_config_entry, 0, sizeof(steering_config_t)); | ||
| memcpy(steer_config_entry, &temp_steer_config, sizeof(steering_config_t)); | ||
| hash_map_put(data->u.decoded.steering_config_map, strdup(temp_steer_config.steering_cfg_id), steer_config_entry); | ||
| steer_config_entry = NULL; |
Comment on lines
5204
to
+5207
| memset(steering_client_entry, 0, sizeof(band_steering_clients_t)); | ||
| memcpy(steering_client_entry, &temp_steering_client, sizeof(band_steering_clients_t)); | ||
| hash_map_put(data->u.decoded.steering_client_map, strdup(temp_steering_client.steering_client_id), steering_client_entry); | ||
| steering_client_entry = NULL; |
Comment on lines
5309
to
+5312
| memset(vif_neighbor_entry, 0, sizeof(vif_neighbors_t)); | ||
| memcpy(vif_neighbor_entry, &temp_vif_neighbor, sizeof(vif_neighbors_t)); | ||
| hash_map_put(data->u.decoded.vif_neighbors_map, strdup(temp_vif_neighbor.neighbor_id), vif_neighbor_entry); | ||
| vif_neighbor_entry = NULL; |
sshah500
force-pushed
the
test_coverity_bkp
branch
from
July 14, 2026 13:25
edb544f to
89a81f9
Compare
Comment on lines
4713
to
+4716
| memset(stat_config_entry, 0, sizeof(stats_config_t)); | ||
| memcpy(stat_config_entry, &temp_stat_config_entry, sizeof(stats_config_t)); | ||
| hash_map_put(data->u.decoded.stats_config_map, strdup(temp_stat_config_entry.stats_cfg_id), stat_config_entry); | ||
| stat_config_entry = NULL; |
Comment on lines
4852
to
+4855
| memset(steer_config_entry, 0, sizeof(steering_config_t)); | ||
| memcpy(steer_config_entry, &temp_steer_config, sizeof(steering_config_t)); | ||
| hash_map_put(data->u.decoded.steering_config_map, strdup(temp_steer_config.steering_cfg_id), steer_config_entry); | ||
| steer_config_entry = NULL; |
Comment on lines
5204
to
+5207
| memset(steering_client_entry, 0, sizeof(band_steering_clients_t)); | ||
| memcpy(steering_client_entry, &temp_steering_client, sizeof(band_steering_clients_t)); | ||
| hash_map_put(data->u.decoded.steering_client_map, strdup(temp_steering_client.steering_client_id), steering_client_entry); | ||
| steering_client_entry = NULL; |
Comment on lines
5309
to
+5312
| memset(vif_neighbor_entry, 0, sizeof(vif_neighbors_t)); | ||
| memcpy(vif_neighbor_entry, &temp_vif_neighbor, sizeof(vif_neighbors_t)); | ||
| hash_map_put(data->u.decoded.vif_neighbors_map, strdup(temp_vif_neighbor.neighbor_id), vif_neighbor_entry); | ||
| vif_neighbor_entry = NULL; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: Fixing Vulnerability issues.
Test Procedure: Build should be successful and the regression test should also succeed
Risks: Low
Priority: P1
Signed-off-by: sachinkumar_shah@comcast.com