diff options
author | Guy Harris <guy@alum.mit.edu> | 2015-08-07 15:27:11 -0700 |
---|---|---|
committer | Guy Harris <guy@alum.mit.edu> | 2015-08-07 22:28:20 +0000 |
commit | a001ed6f8a88d1870e1377fae3c73b4b5e7160e3 (patch) | |
tree | 19d616a13db23d122f5fa301f831f2f656e517a1 /wiretap | |
parent | 817d9bd621caf809b9e6c6d93ab26d7d4f76a33f (diff) |
Avoid (unlikely) NRB record size overflows.
If a host name is *so* long that an entry for it won't fit in a
65535-byte Name Resolution Block record, ignore the entry for that host.
Use more appropriate data types (guint32 for sizes that are 32-bit
unsigned integers, guint16 for the host name length as it'd better fit
in 16 bits).
Clean up some comments.
Remove a _U_ that's applied to a variable that *is* used.
Change-Id: I153d5aa885105149c62a5e5d2b78b54cf6ed7b4e
Reviewed-on: https://code.wireshark.org/review/9917
Reviewed-by: Guy Harris <guy@alum.mit.edu>
Diffstat (limited to 'wiretap')
-rw-r--r-- | wiretap/pcapng.c | 51 |
1 files changed, 42 insertions, 9 deletions
diff --git a/wiretap/pcapng.c b/wiretap/pcapng.c index 8160dd1c04..6d1cc49d67 100644 --- a/wiretap/pcapng.c +++ b/wiretap/pcapng.c @@ -3608,7 +3608,10 @@ pcapng_write_name_resolution_block(wtap_dumper *wdh, int *err) pcapng_block_header_t bh; pcapng_name_resolution_block_t nrb; guint8 *rec_data; - gint rec_off, namelen, tot_rec_len; + guint32 rec_off; + size_t hostnamelen; + guint16 namelen; + guint32 tot_rec_len; hashipv4_t *ipv4_hash_list_entry; hashipv6_t *ipv6_hash_list_entry; int i; @@ -3628,7 +3631,16 @@ pcapng_write_name_resolution_block(wtap_dumper *wdh, int *err) while(ipv4_hash_list_entry != NULL){ nrb.record_type = NRES_IP4RECORD; - namelen = (gint)strlen(ipv4_hash_list_entry->name) + 1; + hostnamelen = strlen(ipv4_hash_list_entry->name); + if (hostnamelen > (G_MAXUINT16 - 4) - 1) { + /* + * This won't fit in a maximum-sized record; discard it. + */ + i++; + ipv4_hash_list_entry = (hashipv4_t *)g_list_nth_data(wdh->addrinfo_lists->ipv4_addr_list, i); + continue; + } + namelen = (guint16)(hostnamelen + 1); nrb.record_len = 4 + namelen; tot_rec_len = 4 + nrb.record_len + PADDING4(nrb.record_len); @@ -3681,13 +3693,28 @@ pcapng_write_name_resolution_block(wtap_dumper *wdh, int *err) while(ipv6_hash_list_entry != NULL){ nrb.record_type = NRES_IP6RECORD; - namelen = (gint)strlen(ipv6_hash_list_entry->name) + 1; + hostnamelen = strlen(ipv6_hash_list_entry->name); + if (hostnamelen > (G_MAXUINT16 - 16) - 1) { + /* + * This won't fit in a maximum-sized record; discard it. + */ + i++; + ipv6_hash_list_entry = (hashipv6_t *)g_list_nth_data(wdh->addrinfo_lists->ipv6_addr_list, i); + continue; + } + namelen = (guint16)(hostnamelen + 1); nrb.record_len = 16 + namelen; /* 16 bytes IPv6 address length */ /* 2 bytes record type, 2 bytes length field */ tot_rec_len = 4 + nrb.record_len + PADDING4(nrb.record_len); if (rec_off + tot_rec_len > NRES_REC_MAX_SIZE){ - /* We know the total length now; copy the block header. */ + /* + * This record would overflow our maximum size for Name + * Resolution Blocks; write out all the records we created + * before it, and start a new NRB. + */ + + /* First, copy the block header. */ memcpy(rec_data, &bh, sizeof(bh)); /* End of record */ @@ -3736,7 +3763,7 @@ pcapng_write_name_resolution_block(wtap_dumper *wdh, int *err) struct option option_hdr; guint32 comment_len = 0, comment_pad_len = 0; wtapng_name_res_t *nrb_hdr = wdh->nrb_hdr; - gint prev_rec_off = rec_off; + guint32 prev_rec_off = rec_off; /* get lengths first to make sure we can fit this into the block */ if (nrb_hdr->opt_comment) { @@ -3755,7 +3782,13 @@ pcapng_write_name_resolution_block(wtap_dumper *wdh, int *err) options_total_length += 4; if (rec_off + options_total_length > NRES_REC_MAX_SIZE) { - /* Too much; copy the block header. */ + /* + * This record would overflow our maximum size for Name + * Resolution Blocks; write out all the records we created + * before it, and start a new NRB. + */ + + /* First, copy the block header. */ memcpy(rec_data, &bh, sizeof(bh)); /* End of record */ @@ -3785,7 +3818,7 @@ pcapng_write_name_resolution_block(wtap_dumper *wdh, int *err) option_hdr.value_length = comment_len; memcpy(rec_data + rec_off, &option_hdr, sizeof(option_hdr)); - rec_off += (gint)sizeof(option_hdr); + rec_off += (guint32)sizeof(option_hdr); /* Write the comments string */ memcpy(rec_data + rec_off, nrb_hdr->opt_comment, comment_len); @@ -3803,7 +3836,7 @@ pcapng_write_name_resolution_block(wtap_dumper *wdh, int *err) rec_off += 4; /* sanity check */ - g_assert((gint)options_total_length == rec_off - prev_rec_off); + g_assert(options_total_length == rec_off - prev_rec_off); } } @@ -3882,7 +3915,7 @@ static gboolean pcapng_dump(wtap_dumper *wdh, /* Finish writing to a dump file. Returns TRUE on success, FALSE on failure. */ -static gboolean pcapng_dump_close(wtap_dumper *wdh, int *err _U_) +static gboolean pcapng_dump_close(wtap_dumper *wdh, int *err) { guint i, j; |