From 019aeca364cf467b55dff4a4eac4e16568e1ef24 Mon Sep 17 00:00:00 2001 From: John Thacker Date: Sun, 15 Mar 2026 02:44:15 +0000 Subject: [PATCH] K12: Fix a possible stack overflow when writing, and print error Check the length of a source descriptor record when writing a K12 file. Convert a function from g_hash_table_foreach to using a GHashTableIter in order to address an concern about error handling already mentioned in a comment. Thank to bcoles for the POC Fix #21094 AI-Assisted: yes (Claude found the POC and suggested a solution, which I verified and extended.) (backported from commit a79be0f589c945233239aa30b74772fab086cfec) Co-authored-by: John Thacker --- wiretap/k12.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) Index: wireshark-3.6.24/wiretap/k12.c =================================================================== --- wireshark-3.6.24.orig/wiretap/k12.c +++ wireshark-3.6.24/wiretap/k12.c @@ -200,6 +200,14 @@ static const guint8 k12_file_magic[] = { #define K12_FILE_HDR_RECORD_COUNT_2 0x2C #define K12_FILE_BLOB_LEN 16 + /* + * Where the next unknown 16 bytes are stuffed to the file. + * Following the file header, they appear every 8192 bytes, + * starting right after the file header, so if the file offset + * relative to the file header is a multiple of 8192, the + * 16-byte blob is there. + */ +#define K12_RECORD_SIZE 8192 typedef struct { guint32 file_len; @@ -1155,13 +1163,10 @@ static gboolean k12_dump_record(wtap_dum return TRUE; } -static void k12_dump_src_setting(gpointer k _U_, gpointer v, gpointer p) { - k12_src_desc_t* src_desc = (k12_src_desc_t*)v; - wtap_dumper *wdh = (wtap_dumper *)p; +static gboolean k12_dump_src_setting(k12_src_desc_t *src_desc, wtap_dumper *wdh, int *err, char **err_info) { guint32 len; guint offset; guint i; - int errxxx; /* dummy */ union { guint8 buffer[8192]; @@ -1245,6 +1250,15 @@ static void k12_dump_src_setting(gpointe break; } + size_t total_len = (size_t)offset + obj.record.name_len + obj.record.stack_len; + + if (total_len > K12_RECORD_SIZE) { + *err = WTAP_ERR_UNWRITABLE_REC_DATA; + *err_info = g_strdup_printf("k12: source descriptor record length %zu > %u", + total_len, K12_RECORD_SIZE); + return FALSE; + } + memcpy(obj.buffer + offset, src_desc->input_name, obj.record.name_len); @@ -1260,7 +1274,7 @@ static void k12_dump_src_setting(gpointe obj.record.name_len = g_htons(obj.record.name_len); obj.record.stack_len = g_htons(obj.record.stack_len); - k12_dump_record(wdh,len,obj.buffer, &errxxx); /* fwrite errs ignored: see k12_dump below */ + return k12_dump_record(wdh,len,obj.buffer, err); } static gboolean k12_dump(wtap_dumper *wdh, const wtap_rec *rec, @@ -1301,12 +1315,16 @@ static gboolean k12_dump(wtap_dumper *wd if (k12->num_of_records == 0) { k12_t* file_data = (k12_t*)pseudo_header->k12.stuff; - /* XXX: We'll assume that any fwrite errors in k12_dump_src_setting will */ - /* repeat during the final k12_dump_record at the end of k12_dump */ - /* (and thus cause an error return from k12_dump). */ - /* (I don't see a reasonably clean way to handle any fwrite errors */ - /* encountered in k12_dump_src_setting). */ - g_hash_table_foreach(file_data->src_by_id,k12_dump_src_setting,wdh); + GHashTableIter iter; + void *value; + k12_src_desc_t *src_desc; + g_hash_table_iter_init(&iter, file_data->src_by_id); + while (g_hash_table_iter_next(&iter, NULL, &value)) { + src_desc = (k12_src_desc_t*)value; + if (!k12_dump_src_setting(src_desc, wdh, err, err_info)) { + return FALSE; + } + } } obj.record.len = 0x20 + rec->rec_header.packet_header.caplen; obj.record.len += (obj.record.len % 4) ? 4 - obj.record.len % 4 : 0;