RDKB-65853: Fixing Vulnerability issue#1259
Open
sshah500 wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets vulnerability-related reliability issues by tightening memory management in OVSDB-to-RDK translation paths and RBUS handler encoding paths.
Changes:
- Added cleanup paths in multiple OVSDB translation functions to free partially built hash-maps/entries on failure.
- Updated
apply_ignite_config()to allocatewebconfig_subdoc_data_ton the heap and free it appropriately. - Fixed a success-path leak in
set_force_vap_apply()by freeing encoded webconfig data before freeing the container.
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 error cleanup to avoid leaking partially constructed decoded maps during OVSDB translation. |
| source/core/wifi_ctrl_rbus_handlers.c | Adjusts webconfig subdoc allocation/freeing and plugs a success-path encoded-data leak. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4667
to
+4670
| stats_config_t *stat_config_entry; | ||
| stats_config_t temp_stat_config_entry; | ||
| unsigned int i = 0; | ||
| webconfig_error_t rc = webconfig_error_none; |
| @@ -4667,6 +4667,7 @@ webconfig_error_t translate_config_from_ovsdb_for_stats_config(webconfig_subdoc | |||
| stats_config_t *stat_config_entry; | |||
| @@ -5120,6 +5159,7 @@ webconfig_error_t translate_config_from_ovsdb_for_steering_clients(webconfig_su | |||
| band_steering_clients_t *steering_client_entry; | |||
| @@ -5203,6 +5262,7 @@ webconfig_error_t translate_config_from_ovsdb_for_vif_neighbors(webconfig_subdo | |||
| vif_neighbors_t *vif_neighbor_entry; | |||
| @@ -5203,6 +5262,7 @@ webconfig_error_t translate_config_from_ovsdb_for_vif_neighbors(webconfig_subdo | |||
| vif_neighbors_t *vif_neighbor_entry; | |||
| @@ -5203,6 +5262,7 @@ | |||
| vif_neighbors_t *vif_neighbor_entry; | |||
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
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.
RDKB-65853: Fixing Vulnerability issue
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