Wiretap file open routines should not free wth->priv on error, since that
authorEvan Huus <eapache@gmail.com>
Mon, 25 Mar 2013 22:04:15 +0000 (22:04 -0000)
committerEvan Huus <eapache@gmail.com>
Mon, 25 Mar 2013 22:04:15 +0000 (22:04 -0000)
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

wiretap/ascendtext.c
wiretap/file_access.c
wiretap/lanalyzer.c
wiretap/libpcap.c
wiretap/netmon.c
wiretap/nettl.c
wiretap/netxray.c

index bac0cccb0ed251cb752e199633ad229605ecfeb2..d0b4e765c659e9c29c92e4b27a0163d3cccc8cd1 100644 (file)
@@ -225,7 +225,6 @@ int ascend_open(wtap *wth, int *err, gchar **err_info)
      offset that we can apply to each packet.
    */
   if (wtap_fstat(wth, &statbuf, err) == -1) {
-    g_free(ascend);
     return -1;
   }
   ascend->inittime = statbuf.st_ctime;
index 8b9094498bfe2fd85a253f8f58782bdde44eca34..44528c1a6a76877db8a80c7278455554aa42b4bd 100644 (file)
  * 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.
  *
+ * 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
  * possible that you could have captured someone a router telnet-session
  * using another tool. So, a libpcap trace of an toshiba "snoop" session
index c3ed8f2efaa9eccacf0577cbf5b3b71cde568d18..e8335c91eaaf47191d6b1c362356ad400f8ebb81 100644 (file)
@@ -358,8 +358,6 @@ int lanalyzer_open(wtap *wth, int *err, gchar **err_info)
                        *err = file_error(wth->fh, err_info);
                        if (*err == 0)
                                *err = WTAP_ERR_SHORT_READ;
-                       g_free(wth->priv);
-                       wth->priv = NULL;
                        return -1;
                }
 
@@ -377,8 +375,6 @@ int lanalyzer_open(wtap *wth, int *err, gchar **err_info)
                                        *err = file_error(wth->fh, err_info);
                                        if (*err == 0)
                                                *err = WTAP_ERR_SHORT_READ;
-                                       g_free(wth->priv);
-                                       wth->priv = NULL;
                                        return -1;
                                }
 
@@ -419,8 +415,6 @@ int lanalyzer_open(wtap *wth, int *err, gchar **err_info)
                                                wth->file_encap = WTAP_ENCAP_TOKEN_RING;
                                                break;
                                        default:
-                                               g_free(wth->priv);
-                                               wth->priv = NULL;
                                                *err = WTAP_ERR_UNSUPPORTED_ENCAP;
                                                *err_info = g_strdup_printf("lanalyzer: board type %u unknown",
                                                    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
                                 * can read this header */
                                if (file_seek(wth->fh, -LA_RecordHeaderSize, SEEK_CUR, err) == -1) {
-                                       g_free(wth->priv);
-                                       wth->priv = NULL;
                                        return -1;
                                }
                                return 1;
 
                        default:
                                if (file_seek(wth->fh, record_length, SEEK_CUR, err) == -1) {
-                                       g_free(wth->priv);
-                                       wth->priv = NULL;
                                        return -1;
                                }
                                break;
index 018a35dbd359e5a0753f646dcc2c0b6ae3c3400a..cb9e97cf59a3469811e793078988fa45f99dac31 100644 (file)
@@ -364,7 +364,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                         * Well, we couldn't even read it.
                         * Give up.
                         */
-                       g_free(wth->priv);
                        return -1;
 
                case THIS_FORMAT:
@@ -373,7 +372,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                         * Put the seek pointer back, and finish.
                         */
                        if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
-                               g_free(wth->priv);
                                return -1;
                        }
                        goto done;
@@ -394,7 +392,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                 */
                wth->file_type = WTAP_FILE_PCAP_SS990915;
                if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
-                       g_free(wth->priv);
                        return -1;
                }
        } else {
@@ -416,7 +413,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                         * Well, we couldn't even read it.
                         * Give up.
                         */
-                       g_free(wth->priv);
                        return -1;
 
                case THIS_FORMAT:
@@ -426,7 +422,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                         * Put the seek pointer back, and finish.
                         */
                        if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
-                               g_free(wth->priv);
                                return -1;
                        }
                        goto done;
@@ -445,7 +440,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                 */
                wth->file_type = WTAP_FILE_PCAP_SS990417;
                if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
-                       g_free(wth->priv);
                        return -1;
                }
                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.
                         * Give up.
                         */
-                       g_free(wth->priv);
                        return -1;
 
                case THIS_FORMAT:
@@ -464,7 +457,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                         * Put the seek pointer back, and finish.
                         */
                        if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
-                               g_free(wth->priv);
                                return -1;
                        }
                        goto done;
@@ -485,7 +477,6 @@ int libpcap_open(wtap *wth, int *err, gchar **err_info)
                 */
                wth->file_type = WTAP_FILE_PCAP_NOKIA;
                if (file_seek(wth->fh, first_packet_offset, SEEK_SET, err) == -1) {
-                       g_free(wth->priv);
                        return -1;
                }
        }
index 0a7632475acdf595cab08cf67354bfd83c976ede..b026ae385fd35bef78f47d8e37e48c38f5362338 100644 (file)
@@ -329,14 +329,12 @@ int netmon_open(wtap *wth, int *err, gchar **err_info)
                *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",
                    frame_table_length);
-               g_free(netmon);
                return -1;
        }
        if (frame_table_size == 0) {
                *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",
                    frame_table_length);
-               g_free(netmon);
                return -1;
        }
        /*
@@ -356,11 +354,9 @@ int netmon_open(wtap *wth, int *err, gchar **err_info)
                *err = WTAP_ERR_BAD_FILE;
                *err_info = g_strdup_printf("netmon: frame table length is %u, which is larger than we support",
                    frame_table_length);
-               g_free(netmon);
                return -1;
        }
        if (file_seek(wth->fh, frame_table_offset, SEEK_SET, err) == -1) {
-               g_free(netmon);
                return -1;
        }
        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)
                        *err = WTAP_ERR_SHORT_READ;
                g_free(frame_table);
-               g_free(netmon);
                return -1;
        }
        netmon->frame_table_size = frame_table_size;
index 1fa4034f24ddf5abe15027f8d7e24a7b7d4f884a..564644426eaabf6758b1ecb0da529b38eae07703 100644 (file)
@@ -241,18 +241,12 @@ int nettl_open(wtap *wth, int *err, gchar **err_info)
     bytes_read = file_read(dummy, 4, wth->fh);
     if (bytes_read != 4) {
         if (*err != 0) {
-            wth->priv = NULL;
-            g_free(nettl);
             return -1;
         }
         if (bytes_read != 0) {
             *err = WTAP_ERR_SHORT_READ;
-            wth->priv = NULL;
-            g_free(nettl);
             return -1;
         }
-        wth->priv = NULL;
-        g_free(nettl);
         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) {
-        g_free(nettl);
        return -1;
     }
     wth->tsprecision = WTAP_FILE_TSPREC_USEC;
index 8acf26a86e8e5c08bb2f9dde6590c223895101ab..6fb6bbc1e062b4634f4c9aec621802acd139b70e 100644 (file)
@@ -901,7 +901,6 @@ int netxray_open(wtap *wth, int *err, gchar **err_info)
 
        /* Seek to the beginning of the data records. */
        if (file_seek(wth->fh, netxray->start_offset, SEEK_SET, err) == -1) {
-               g_free(netxray);
                return -1;
        }