https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7824
Summary: Failure to check return values from ws_fopen calls
Product: Wireshark
Version: 1.8.2
Platform: All
OS/Version: All
Status: NEW
Severity: Major
Priority: Low
Component: Common utilities (libwsutil)
AssignedTo: bugzilla-admin@xxxxxxxxxxxxx
ReportedBy: wp02855@xxxxxxxxx
Created attachment 9319
--> https://bugs.wireshark.org/bugzilla/attachment.cgi?id=9319
contains all the patch files (diff -u format) listed in the description section
Build Information:
odie:/usr/local/src/wireshark-1.8.2 # ./wireshark -v
wireshark 1.8.2 (SVN Rev Unknown from unknown)
Copyright 1998-2012 Gerald Combs <gerald@xxxxxxxxxxxxx> and contributors.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Compiled (64-bit) with GTK+ 2.22.1, with Cairo 1.10.2, with Pango 1.28.3, with
GLib 2.28.0, with libpcap, with libz 1.2.5, without POSIX capabilities, without
SMI, without c-ares, without ADNS, without Lua, without Python, with GnuTLS
2.8.6, with Gcrypt 1.4.6, with MIT Kerberos, without GeoIP, without PortAudio,
with AirPcap.
Running on Linux 2.6.37.6-0.7-desktop, with locale POSIX, with libpcap version
1.1.1, with libz 1.2.5, GnuTLS 2.8.6, Gcrypt 1.4.6, without AirPcap.
Built using gcc 4.5.1 20101208 [gcc-4_5-branch revision 167585].
--
In reviewing calls to ws_fopen(), I found instances where the
return value of ws_fopen is not checked for the NULL condition,
which indicates failure of ws_fopen() in wireshark-1.8.2/1.8.3
In directory epan, file 'diam_cict.l', the value of fh is
not checked for a NULL condition after a call to ws_fopen,
this patch adds in a sanity check to the code in question.
Patch is below and attached to this bug report:
odie:/usr/local/src/wireshark-1.8.2/epan # diff -u diam_dict.l.orig diam_dict.l
--- diam_dict.l.orig 2012-10-09 06:02:52.061547597 -0700
+++ diam_dict.l 2012-10-09 08:26:33.753653037 -0700
@@ -607,11 +607,15 @@
fh = ws_fopen(fname,"r");
+ if (fh == NULL) {
+ D(("unable to open file: %s for reading, fh: %p\n", fname,
fh));
+ return NULL;
+ }
+
D(("fname: %s fh: %p\n",fname,fh));
g_free(fname);
-
return fh;
}
The same issue occurs in directory wireshark-1.8.2/plugins/wimaxasncp
file wimaxasncp_dict.c where 'fh' is assigned to a ws_fopen call, but
no check is made to determine if a NULL value is assigned to 'fh'
which would indicate failure.
this patch adds in a sanity check to the code in question.
Patch is below and attached to this bug report:
--- wimaxasncp_dict.c.orig 2012-10-09 07:38:10.679869428 -0700
+++ wimaxasncp_dict.c 2012-10-09 08:33:29.602598815 -0700
@@ -2921,6 +2921,10 @@
}
fh = ws_fopen(fname,"r");
+ if (fh == NULL) {
+ D(("unable to open file: %s for reading, fh: %p\n", fname,
fh));
+ return NULL;
+ }
D(("fname: %s fh: %p\n",fname,fh));
The same issue occurs in directory wireshark-1.8.2/plugins/wimaxasncp
file wimaxasncp_dict.l where 'fh' is assigned to a ws_fopen call, but
no check is made to determine if a NULL value is assigned to 'fh'
which would indicate failure.
this patch adds in a sanity check to the code in question.
Patch is below and attached to this bug report:
--- wimaxasncp_dict.l.orig 2012-10-09 07:46:54.148186118 -0700
+++ wimaxasncp_dict.l 2012-10-09 08:34:57.868926566 -0700
@@ -594,6 +594,10 @@
}
fh = ws_fopen(fname,"r");
+ if (fh == NULL) {
+ D(("unable to open file: %s for reading, fh: %p\n", fname,
fh));
+ return NULL;
+ }
D(("fname: %s fh: %p\n",fname,fh));
The same issue occurs in directory wireshark-1.8.2/plugins/wimaxasncp
file packet-asn1.c where 'namelist' is assigned to a ws_fopen call, but
no check is made to determine if a NULL value is assigned to 'namelist'
which would indicate failure.
this patch adds in a sanity check to the code in question.
Patch is below and attached to this bug report:
--- packet-asn1.c.orig 2012-10-09 07:54:40.436461125 -0700
+++ packet-asn1.c 2012-10-09 07:56:57.072900547 -0700
@@ -4248,7 +4248,11 @@
G_TYPE_STRING, G_TYPE_STRING);
namelist = ws_fopen("namelist.txt", "w");
- build_tree_view(model, PDUtree, NULL);
+ if (namelist == NULL)
+ fprintf(stderr, "unable to open file: namelist.txt for
writing!\n");
+ else
+ build_tree_view(model, PDUtree, NULL);
+
fclose(namelist);
namelist = 0;
The same issue occurs in directory wireshark-1.8.2/wiretap file
'k12.c' where 'dbg_out' is assigned to a ws_fopen call, but
no check is made to determine if a NULL value is assigned to 'dbg_out'
which would indicate failure.
This code also converts 'unsigned <variable>' to 'unsigned int <variable>'
which was submitted in a previous bug report.
this patch adds in a sanity check to the code in question.
Patch is below and attached to this bug report:
--- k12.c.orig 2012-10-08 16:45:19.188657926 -0700
+++ k12.c 2012-10-09 08:12:53.355959060 -0700
@@ -64,7 +64,7 @@
FILE* dbg_out = NULL;
char* env_file = NULL;
-static unsigned debug_level = 0;
+static unsigned int debug_level = 0;
void k12_fprintf(char* fmt, ...) {
va_list ap;
@@ -81,7 +81,7 @@
fprintf(dbg_out,"\n"); \
} } while(0)
-void k12_hexdump(guint level, gint64 offset, char* label, unsigned char* b,
unsigned len) {
+void k12_hexdump(guint level, gint64 offset, char* label, unsigned char* b,
unsigned int len) {
static const char* c2t[] = {
"00","01","02","03","04","05","06","07","08","09","0a","0b","0c","0d","0e","0f",
"10","11","12","13","14","15","16","17","18","19","1a","1b","1c","1d","1e","1f",
@@ -100,7 +100,7 @@
"e0","e1","e2","e3","e4","e5","e6","e7","e8","e9","ea","eb","ec","ed","ee","ef",
"f0","f1","f2","f3","f4","f5","f6","f7","f8","f9","fa","fb","fc","fd","fe","ff"
};
- unsigned i;
+ unsigned int i;
if (debug_level < level) return;
@@ -682,8 +682,14 @@
#ifdef DEBUG_K12
gchar* env_level = getenv("K12_DEBUG_LEVEL");
env_file = getenv("K12_DEBUG_FILENAME");
- if ( env_file ) dbg_out = ws_fopen(env_file,"w");
- else dbg_out = stderr;
+ if ( env_file ) {
+ dbg_out = ws_fopen(env_file,"w");
+ if (dog_out == NULL) {
+ K12_DBG(1,("unable to open K12 DEBUG FILENAME for writing!"));
+ return -1;
+ }
+ else
+ dbg_out = stderr;
if ( env_level ) debug_level = strtoul(env_level,NULL,10);
K12_DBG(1,("k12_open: ENTER debug_level=%u",debug_level));
#endif
All code compiles cleanly via 'make'.
List of files in tarball ws_fopen-patches.tar.gz:
odie:/usr/local/src/patchfiles/wireshark-1.8.2 # tar tvf
ws_fopen-patches.tar.gz
-rw-r--r-- root/root 1596 2012-10-09 08:19 k12.c.patch
-rw-r--r-- root/root 448 2012-10-09 07:59 packet-asn1.c.patch
-rw-r--r-- root/root 352 2012-10-09 08:27 diam_dict.l.patch
-rw-r--r-- root/root 326 2012-10-09 08:35 wimaxasncp_dict.l.patch
-rw-r--r-- root/root 328 2012-10-09 08:36 wimaxasncp_dict.c.patch
Bill Parker
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
