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 <mmann78@netscape.net>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>

1 files changed, 26 insertions, 5 deletions

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);