From 4f3646fe62244f7aba3f309c509c3414f1028650 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Sat, 14 Feb 2015 15:36:09 +0100 Subject: Fix handling of invalid UAT items If the UAT file failed a field check, then the user_data pointer may be empty. As a result uat_save() triggers an invalid write. (Discovered while working with a dfilter_macros file having duplicate names for bug 10957, caught by ASAN.) The second issue fixed in this patch is that the validity of an item is only calculated when a new record is added. So even if the user edits the UAT and makes the entry valid, it would not be saved. This is solved by adding a new uat_update_record() function which got wires up into GTK and Qt. Some open-coded g_array_index and UAT[_USER]_INDEX_PTR are also converted. Even after this patch, Qt has some issues with UAT handling. In particular, it saves new, but empty/invalid, items. It also it does not check individual fields when saving all fields (unlike Gtk). This patch focused on getting Gtk fixed first so ignores those existing issues. Change-Id: Ia35cfe9d2b793c65144ae7e29a1ed706b6668d99 Reviewed-on: https://code.wireshark.org/review/7120 Petri-Dish: Michael Mann Reviewed-by: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Michael Mann --- epan/uat.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) (limited to 'epan/uat.c') diff --git a/epan/uat.c b/epan/uat.c index 0b4099a6c7..c285aacae5 100644 --- a/epan/uat.c +++ b/epan/uat.c @@ -129,7 +129,7 @@ void* uat_add_record(uat_t* uat, const void* data, gboolean valid_rec) { /* Save a copy of the raw (possibly that may contain invalid field values) data */ g_array_append_vals (uat->raw_data, data, 1); - rec = uat->raw_data->data + (uat->record_size * (uat->raw_data->len-1)); + rec = UAT_INDEX_PTR(uat, uat->raw_data->len - 1); if (uat->copy_cb) { uat->copy_cb(rec, data, (unsigned int) uat->record_size); @@ -139,7 +139,7 @@ void* uat_add_record(uat_t* uat, const void* data, gboolean valid_rec) { /* Add a "known good" record to the list to be used by the dissector */ g_array_append_vals (uat->user_data, data, 1); - rec = uat->user_data->data + (uat->record_size * (uat->user_data->len-1)); + rec = UAT_USER_INDEX_PTR(uat, uat->user_data->len - 1); if (uat->copy_cb) { uat->copy_cb(rec, data, (unsigned int) uat->record_size); @@ -151,12 +151,32 @@ void* uat_add_record(uat_t* uat, const void* data, gboolean valid_rec) { } g_array_append_vals (uat->valid_data, &valid_rec, 1); - valid = (gboolean*)(uat->valid_data->data + (sizeof(gboolean) * (uat->valid_data->len-1))); + valid = &g_array_index(uat->valid_data, gboolean, uat->valid_data->len-1); *valid = valid_rec; return rec; } +/* Updates the validity of a record. */ +void uat_update_record(uat_t *uat, const void *data, gboolean valid_rec) { + guint pos; + gboolean *valid; + + /* Locate internal UAT data pointer. */ + for (pos = 0; pos < uat->raw_data->len; pos++) { + if (UAT_INDEX_PTR(uat, pos) == data) { + break; + } + } + if (pos == uat->raw_data->len) { + /* Data is not within list?! */ + g_assert_not_reached(); + } + + valid = &g_array_index(uat->valid_data, gboolean, pos); + *valid = valid_rec; +} + void uat_swap(uat_t* uat, guint a, guint b) { size_t s = uat->record_size; void* tmp; @@ -316,12 +336,13 @@ gboolean uat_save(uat_t* uat, char** error) { /* Now copy "good" raw_data entries to user_data */ for ( i = 0 ; i < uat->raw_data->len ; i++ ) { - void* rec = uat->raw_data->data + (uat->record_size * i); + void *rec = UAT_INDEX_PTR(uat, i); gboolean* valid = (gboolean*)(uat->valid_data->data + sizeof(gboolean)*i); if (*valid) { g_array_append_vals(uat->user_data, rec, 1); if (uat->copy_cb) { - uat->copy_cb(UAT_USER_INDEX_PTR(uat,i), rec, (unsigned int) uat->record_size); + uat->copy_cb(UAT_USER_INDEX_PTR(uat, uat->user_data->len - 1), + rec, (unsigned int) uat->record_size); } UAT_UPDATE(uat); -- cgit v1.2.3