1berry/wireshark
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Wiretap file open routines should not free wth->priv on error, since that

Browse Source 
leads to a double-free in wtap_close. Fix all the instances I found via
manual code review, and add a brief comment to the list of open routines in
file_access.c

Fixes https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8518

svn path=/trunk/; revision=48552
This commit is contained in:
Evan Huus 2013-03-25 22:04:15 +00:00
parent 93be2ad48a
commit 202680971d
7 changed files with 8 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
wiretap/ascendtext.c
View File

@ -225,7 +225,6 @@ int ascend_open(wtap *wth, int *err, gchar **err_info)
offset that we can apply to each packet. offset that we can apply to each packet.
*/ */
if (wtap_fstat(wth, &statbuf, err) == -1) { if (wtap_fstat(wth, &statbuf, err) == -1) {
g_free(ascend);
return -1; return -1;
} }
ascend->inittime = statbuf.st_ctime; ascend->inittime = statbuf.st_ctime;

8
wiretap/file_access.c
View File

@ -98,6 +98,14 @@
* If the routine handles this type of file, it should set the "file_type" * If the routine handles this type of file, it should set the "file_type"
* field in the "struct wtap" to the type of the file. * field in the "struct wtap" to the type of the file.
* *
* Note that the routine does not have to free the private data pointer on
* error. The caller takes care of that by calling wtap_close on error.
* (See https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8518)
*
* However, the caller does have to free the private data pointer when
* returning 0, since the next file type will be called and will likely
* just overwrite the pointer.
*
* Put the trace files that are merely saved telnet-sessions last, since it's * Put the trace files that are merely saved telnet-sessions last, since it's
* possible that you could have captured someone a router telnet-session * possible that you could have captured someone a router telnet-session
* using another tool. So, a libpcap trace of an toshiba "snoop" session * using another tool. So, a libpcap trace of an toshiba "snoop" session

10
wiretap/lanalyzer.c
View File

@ -358,8 +358,6 @@ int lanalyzer_open(wtap *wth, int *err, gchar **err_info)
*err = file_error(wth->fh, err_info); *err = file_error(wth->fh, err_info);
if (*err == 0) if (*err == 0)
*err = WTAP_ERR_SHORT_READ; *err = WTAP_ERR_SHORT_READ;
g_free(wth->priv);
wth->priv = NULL;
return -1; return -1;
} }
@ -377,8 +375,6 @@ int lanalyzer_open(wtap *wth, int *err, gchar **err_info)
*err = file_error(wth->fh, err_info); *err = file_error(wth->fh, err_info);
if (*err == 0) if (*err == 0)
*err = WTAP_ERR_SHORT_READ; *err = WTAP_ERR_SHORT_READ;
g_free(wth->priv);
wth->priv = NULL;
return -1; return -1;
} }
@ -419,8 +415,6 @@ int lanalyzer_open(wtap *wth, int *err, gchar **err_info)
wth->file_encap = WTAP_ENCAP_TOKEN_RING; wth->file_encap = WTAP_ENCAP_TOKEN_RING;
break; break;
default: default:
g_free(wth->priv);
wth->priv = NULL;
*err = WTAP_ERR_UNSUPPORTED_ENCAP; *err = WTAP_ERR_UNSUPPORTED_ENCAP;
*err_info = g_strdup_printf("lanalyzer: board type %u unknown", *err_info = g_strdup_printf("lanalyzer: board type %u unknown",
board_type); board_type);
@ -433,16 +427,12 @@ int lanalyzer_open(wtap *wth, int *err, gchar **err_info)
/* Go back header number of bytes so that lanalyzer_read /* Go back header number of bytes so that lanalyzer_read
* can read this header */ * can read this header */
if (file_seek(wth->fh, -LA_RecordHeaderSize, SEEK_CUR, err) == -1) { if (file_seek(wth->fh, -LA_RecordHeaderSize, SEEK_CUR, err) == -1) {
g_free(wth->priv);
wth->priv = NULL;
return -1; return -1;
} }
return 1; return 1;
default: default:
if (file_seek(wth->fh, record_length, SEEK_CUR, err) == -1) { if (file_seek(wth->fh, record_length, SEEK_CUR, err) == -1) {
g_free(wth->priv);
wth->priv = NULL;
return -1; return -1;
} }
break; break;

9
wiretap/libpcap.c
View File

@ -364,7 +364,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
* Well, we couldn't even read it. * Well, we couldn't even read it.
* Give up. * Give up.
*/ */
g_free(wth->priv);
return -1; return -1;
case THIS_FORMAT: case THIS_FORMAT:
@ -373,7 +372,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
* Put the seek pointer back, and finish. * Put the seek pointer back, and finish.
*/ */
if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
g_free(wth->priv);
return -1; return -1;
} }
goto done; goto done;
@ -394,7 +392,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
*/ */
wth->file_type = WTAP_FILE_PCAP_SS990915; wth->file_type = WTAP_FILE_PCAP_SS990915;
if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
g_free(wth->priv);
return -1; return -1;
} }
} else { } else {
@ -416,7 +413,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
* Well, we couldn't even read it. * Well, we couldn't even read it.
* Give up. * Give up.
*/ */
g_free(wth->priv);
return -1; return -1;
case THIS_FORMAT: case THIS_FORMAT:
@ -426,7 +422,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
* Put the seek pointer back, and finish. * Put the seek pointer back, and finish.
*/ */
if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
g_free(wth->priv);
return -1; return -1;
} }
goto done; goto done;
@ -445,7 +440,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
*/ */
wth->file_type = WTAP_FILE_PCAP_SS990417; wth->file_type = WTAP_FILE_PCAP_SS990417;
if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
g_free(wth->priv);
return -1; return -1;
} }
switch (libpcap_try(wth, err)) { switch (libpcap_try(wth, err)) {
@ -455,7 +449,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
* Well, we couldn't even read it. * Well, we couldn't even read it.
* Give up. * Give up.
*/ */
g_free(wth->priv);
return -1; return -1;
case THIS_FORMAT: case THIS_FORMAT:
@ -464,7 +457,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
* Put the seek pointer back, and finish. * Put the seek pointer back, and finish.
*/ */
if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
g_free(wth->priv);
return -1; return -1;
} }
goto done; goto done;
@ -485,7 +477,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
*/ */
wth->file_type = WTAP_FILE_PCAP_NOKIA; wth->file_type = WTAP_FILE_PCAP_NOKIA;
if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
g_free(wth->priv);
return -1; return -1;
} }
} }

5
wiretap/netmon.c
View File

@ -329,14 +329,12 @@ int netmon_open(wtap *wth, int *err, gchar **err_info)
*err = WTAP_ERR_BAD_FILE; *err = WTAP_ERR_BAD_FILE;
*err_info = g_strdup_printf("netmon: frame table length is %u, which is not a multiple of the size of an entry", *err_info = g_strdup_printf("netmon: frame table length is %u, which is not a multiple of the size of an entry",
frame_table_length); frame_table_length);
g_free(netmon);
return -1; return -1;
} }
if (frame_table_size == 0) { if (frame_table_size == 0) {
*err = WTAP_ERR_BAD_FILE; *err = WTAP_ERR_BAD_FILE;
*err_info = g_strdup_printf("netmon: frame table length is %u, which means it's less than one entry in size", *err_info = g_strdup_printf("netmon: frame table length is %u, which means it's less than one entry in size",
frame_table_length); frame_table_length);
g_free(netmon);
return -1; return -1;
} }
/* /*
@ -356,11 +354,9 @@ int netmon_open(wtap *wth, int *err, gchar **err_info)
*err = WTAP_ERR_BAD_FILE; *err = WTAP_ERR_BAD_FILE;
*err_info = g_strdup_printf("netmon: frame table length is %u, which is larger than we support", *err_info = g_strdup_printf("netmon: frame table length is %u, which is larger than we support",
frame_table_length); frame_table_length);
g_free(netmon);
return -1; return -1;
} }
if (file_seek(wth->fh, frame_table_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, frame_table_offset, SEEK_SET, err) == -1) {
g_free(netmon);
return -1; return -1;
} }
frame_table = (guint32 *)g_malloc(frame_table_length); frame_table = (guint32 *)g_malloc(frame_table_length);
@ -371,7 +367,6 @@ int netmon_open(wtap *wth, int *err, gchar **err_info)
if (*err == 0) if (*err == 0)
*err = WTAP_ERR_SHORT_READ; *err = WTAP_ERR_SHORT_READ;
g_free(frame_table); g_free(frame_table);
g_free(netmon);
return -1; return -1;
} }
netmon->frame_table_size = frame_table_size; netmon->frame_table_size = frame_table_size;

7
wiretap/nettl.c
View File

@ -241,18 +241,12 @@ int nettl_open(wtap *wth, int *err, gchar **err_info)
bytes_read = file_read(dummy, 4, wth->fh); bytes_read = file_read(dummy, 4, wth->fh);
if (bytes_read != 4) { if (bytes_read != 4) {
if (*err != 0) { if (*err != 0) {
wth->priv = NULL;
g_free(nettl);
return -1; return -1;
} }
if (bytes_read != 0) { if (bytes_read != 0) {
*err = WTAP_ERR_SHORT_READ; *err = WTAP_ERR_SHORT_READ;
wth->priv = NULL;
g_free(nettl);
return -1; return -1;
} }
wth->priv = NULL;
g_free(nettl);
return 0; return 0;
} }
@ -290,7 +284,6 @@ int nettl_open(wtap *wth, int *err, gchar **err_info)
} }
if (file_seek(wth->fh, FILE_HDR_SIZE, SEEK_SET, err) == -1) { if (file_seek(wth->fh, FILE_HDR_SIZE, SEEK_SET, err) == -1) {
g_free(nettl);
return -1; return -1;
} }
wth->tsprecision = WTAP_FILE_TSPREC_USEC; wth->tsprecision = WTAP_FILE_TSPREC_USEC;

1
wiretap/netxray.c
View File

@ -901,7 +901,6 @@ int netxray_open(wtap *wth, int *err, gchar **err_info)
/* Seek to the beginning of the data records. */ /* Seek to the beginning of the data records. */
if (file_seek(wth->fh, netxray->start_offset, SEEK_SET, err) == -1) { if (file_seek(wth->fh, netxray->start_offset, SEEK_SET, err) == -1) {
g_free(netxray);
return -1; return -1;
} }