Bug#550191: wireshark 1.0.2-3+lenny6 security fixes
Hi,
Moritz proposed to upload fixes for DoS only security problems to
stable and handle onnly more serious problems via stable-security:
> On Monday 06 July 2009 20:42:21 Moritz Muehlenhoff wrote:
>> On Wed, Jul 01, 2009 at 03:36:44PM -0700, Bálint Réczey wrote:
>> > Hi,
>> >
>> > Wireshark 1.0.8 fixes CVE-2009-1829 and contain other changes fixing
>> > crashes and one fix for a memory leak.
>> >
...
>> Traditionally we've been treating Wireshark crashes triggerable by
>> network traffic as security issues, since someone could use tshark
>> as a networking monitoring/intrusion detection tool. OTOH, both
>> Wireshark's security record and the mere concept (analysing network
>> traffic in a flaky implementation language like C) make this an
>> impractical approach. I would like to propose to document in a file
>> like README.Debian or README.Debian.security that Wireshark is
>> great tool to analyse traffic patterns, but that crashes cannot be
>> ruled out due to the complex nature of the task. Thus, it should
>> not be deployed in scenarios where used for live network monitoring
>> and live pure crash bugs unfixed. Of course all bugs which could
>> trigger code injection will still be fixed in regular DSAs.
>> Additionally we could talk to the stable release managers to allow
>> the latest Wireshark point updates for each stable point update
>> (since the QA done by upstream is quite good). There are similar
>> exceptions already done for some packages, e.g. PostgreSQL.
>
> I support this approach.
>
> Joost
>
The original suggestion was to upload full Wireshark releases from the
stable and old stable Wireshark maintenance branches, but later we
chose to extract the security related fixes and add only those to the
Debian package.
According to that plan I would like to upload the package to "stable"
and I corrected the attached patch to reflect this.
Thanks,
Balint
Index: debian/changelog
===================================================================
--- debian/changelog (revision 13620)
+++ debian/changelog (revision 14468)
@@ -1,3 +1,13 @@
+wireshark (1.0.2-3+lenny6) stable; urgency=high
+
+ * security fixes from Wireshark 1.0.8 and 1.0.9:
+ - The PCNFSD dissector could crash (CVE-2009-1829)
+ - The AFS dissector could crash (CVE-2009-2562)
+ - The OpcUa dissector could use excessive CPU and memory (CVE-2009-3241)
+ (Closes: #533347)
+
+ -- Balint Reczey <balint@balintreczey.hu> Mon, 28 Sep 2009 13:05:13 +0100
+
wireshark (1.0.2-3+lenny5) stable-security; urgency=high
* Security fixes from Wireshark 1.0.7
Index: debian/patches/34_fix_opcua_lockup.dpatch
===================================================================
--- debian/patches/34_fix_opcua_lockup.dpatch (revision 0)
+++ debian/patches/34_fix_opcua_lockup.dpatch (revision 14468)
@@ -0,0 +1,256 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## 34_fix_opcua_lockup.dpatch by <balint@balintreczey.hu>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: Fix excessive CPU and memory use in OpcUa disssector
+
+@DPATCH@
+
+Index: trunk/plugins/opcua/opcua_simpletypes.c
+===================================================================
+--- trunk/plugins/opcua/opcua_simpletypes.c (revision 29828)
++++ trunk/plugins/opcua/opcua_simpletypes.c (revision 29829)
+@@ -34,9 +34,6 @@
+ #include <string.h>
+ #include <epan/emem.h>
+
+-/* string buffer */
+-#define MAX_BUFFER 256
+-
+ #define DIAGNOSTICINFO_ENCODINGMASK_SYMBOLICID_FLAG 0x01
+ #define DIAGNOSTICINFO_ENCODINGMASK_NAMESPACE_FLAG 0x02
+ #define DIAGNOSTICINFO_ENCODINGMASK_LOCALIZEDTEXT_FLAG 0x04
+@@ -53,6 +50,9 @@
+ #define EXTOBJ_ENCODINGMASK_BINBODY_FLAG 0x01
+ #define EXTOBJ_ENCODINGMASK_XMLBODY_FLAG 0x02
+
++/* Chosen arbitrarily */
++#define MAX_ARRAY_LEN 10000
++
+ static int hf_opcua_diag_mask_symbolicflag = -1;
+ static int hf_opcua_diag_mask_namespaceflag = -1;
+ static int hf_opcua_diag_mask_localizedtextflag = -1;
+@@ -317,35 +317,28 @@
+
+ void parseString(proto_tree *tree, tvbuff_t *tvb, gint *pOffset, int hfIndex)
+ {
+- char *szValue = ep_alloc(MAX_BUFFER);
++ char *szValue;
+ gint iOffset = *pOffset;
+ gint32 iLen = tvb_get_letohl(tvb, *pOffset);
+ iOffset+=4;
+
+- if (szValue)
++ if (iLen == -1)
+ {
+- if (iLen == -1)
+- {
+- g_snprintf(szValue, MAX_BUFFER, "[OpcUa Null String]");
+- }
+- else if (iLen >= 0)
+- {
+- int iStrLen = iLen;
+- if (iStrLen > (MAX_BUFFER-1)) iStrLen = MAX_BUFFER - 1;
+- /* copy non null terminated string of length iStrlen */
+- strncpy(szValue, (char*)&tvb->real_data[iOffset], iStrLen);
+- /* set null terminator */
+- szValue[iStrLen] = 0;
+- iOffset += iLen; /* eat the whole string */
+- }
+- else
+- {
+- g_snprintf(szValue, MAX_BUFFER, "[Invalid String] Ups, something is wrong with this message.");
+- }
+-
++ proto_tree_add_string(tree, hfIndex, tvb, *pOffset, (iOffset - *pOffset),
++ "[OpcUa Null String]");
++ }
++ else if (iLen >= 0)
++ {
++ iOffset += iLen; /* eat the whole string */
++ proto_tree_add_item(tree, hfIndex, tvb, *pOffset, (iOffset - *pOffset), TRUE);
++ }
++ else
++ {
++ szValue = ep_strdup_printf("[Invalid String] Invalid length: %d", iLen);
+ proto_tree_add_string(tree, hfIndex, tvb, *pOffset, (iOffset - *pOffset), szValue);
+- *pOffset = iOffset;
+ }
++
++ *pOffset = iOffset;
+ }
+
+ void parseStatusCode(proto_tree *tree, tvbuff_t *tvb, gint *pOffset, int hfIndex)
+@@ -623,11 +616,17 @@
+ /* read array length */
+ iLen = tvb_get_letohl(tvb, *pOffset);
+ proto_tree_add_item(subtree, hf_opcua_ArraySize, tvb, *pOffset, 4, TRUE);
+- *pOffset += 4;
+
+ if (iLen == -1) return; /* no array */
+ if (iLen == 0) return; /* array with zero elements*/
+
++ if (iLen > MAX_ARRAY_LEN)
++ {
++ PROTO_ITEM_SET_GENERATED(proto_tree_add_text(tree, tvb, *pOffset, 4, "Array length %d too large to process", iLen));
++ return;
++ }
++
++ *pOffset += 4;
+ for (i=0; i<iLen; i++)
+ {
+ (*pParserFunction)(subtree, tvb, pOffset, hfIndex);
+@@ -649,11 +648,17 @@
+ /* read array length */
+ iLen = tvb_get_letohl(tvb, *pOffset);
+ proto_tree_add_item(subtree, hf_opcua_ArraySize, tvb, *pOffset, 4, TRUE);
+- *pOffset += 4;
+
+ if (iLen == -1) return; /* no array */
+ if (iLen == 0) return; /* array with zero elements*/
+
++ if (iLen > MAX_ARRAY_LEN)
++ {
++ PROTO_ITEM_SET_GENERATED(proto_tree_add_text(tree, tvb, *pOffset, 4, "Array length %d too large to process", iLen));
++ return;
++ }
++
++ *pOffset += 4;
+ for (i=0; i<iLen; i++)
+ {
+ (*pParserFunction)(subtree, tvb, pOffset);
+@@ -674,11 +679,17 @@
+ /* read array length */
+ iLen = tvb_get_letohl(tvb, *pOffset);
+ proto_tree_add_item(subtree, hf_opcua_ArraySize, tvb, *pOffset, 4, TRUE);
+- *pOffset += 4;
+
+ if (iLen == -1) return; /* no array */
+ if (iLen == 0) return; /* array with zero elements*/
+
++ if (iLen > MAX_ARRAY_LEN)
++ {
++ PROTO_ITEM_SET_GENERATED(proto_tree_add_text(tree, tvb, *pOffset, 4, "Array length %d too large to process", iLen));
++ return;
++ }
++
++ *pOffset += 4;
+ for (i=0; i<iLen; i++)
+ {
+ char szNum[20];
+Index: plugins/opcua/opcua.c
+===================================================================
+--- trunk/plugins/opcua/opcua.c (revision 29828)
++++ trunk/plugins/opcua/opcua.c (revision 29829)
+@@ -184,11 +184,11 @@
+ }
+
+ /* parse message type */
+- if (tvb->real_data[0] == 'U' && tvb->real_data[1] == 'A')
++ if (tvb_get_guint8(tvb, 0) == 'U' && tvb_get_guint8(tvb, 1) == 'A')
+ {
+- if (tvb->real_data[2] == 'T')
++ if (tvb_get_guint8(tvb, 2) == 'T')
+ {
+- switch(tvb->real_data[3])
++ switch(tvb_get_guint8(tvb, 3))
+ {
+ case 'H': msgtype = MSG_HELLO;
+ pfctParse = parseHello;
+@@ -203,9 +203,9 @@
+ break;
+ }
+ }
+- else if (tvb->real_data[2] == 'M')
++ else if (tvb_get_guint8(tvb, 2) == 'M')
+ {
+- switch(tvb->real_data[3])
++ switch(tvb_get_guint8(tvb, 3))
+ {
+ case 'G': msgtype = MSG_DATA_LAST_CHUNK;
+ pfctParse = parseData;
+Index: plugins/opcua/opcua_transport_layer.c
+===================================================================
+--- trunk/plugins/opcua/opcua_transport_layer.c (revision 29828)
++++ trunk/plugins/opcua/opcua_transport_layer.c (revision 29829)
+@@ -107,34 +107,10 @@
+ proto_register_field_array(proto, hf, array_length(hf));
+ }
+
+-/** helper functions for adding strings,
+- * that are not zero terminated.
+- */
+-void addString(proto_tree *tree,
+- int hfindex,
+- tvbuff_t *tvb,
+- gint start,
+- gint length,
+- const char *value)
+-{
+- char *szValue = ep_alloc(256);
+-
+- if (szValue)
+- {
+- if (length > 255) length = 255;
+- /* copy non null terminated string data */
+- strncpy(szValue, value, length);
+- /* set null terminator */
+- szValue[length] = 0;
+-
+- proto_tree_add_string(tree, hfindex, tvb, start, length, szValue);
+- }
+-}
+-
+ /* Transport Layer: message parsers */
+ void parseHello(proto_tree *tree, tvbuff_t *tvb, gint *pOffset)
+ {
+- addString(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, tvb->real_data); *pOffset+=4;
++ proto_tree_add_item(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_len, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_ver, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_cid, tvb, *pOffset, 16, TRUE); *pOffset+=16;
+@@ -146,7 +122,7 @@
+
+ void parseAcknowledge(proto_tree *tree, tvbuff_t *tvb, gint *pOffset)
+ {
+- addString(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, tvb->real_data); *pOffset+=4;
++ proto_tree_add_item(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_len, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_cid, tvb, *pOffset, 16, TRUE); *pOffset+=16;
+ proto_tree_add_item(tree, hf_opcua_transport_rlifetime, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+@@ -157,7 +133,7 @@
+
+ void parseDisconnect(proto_tree *tree, tvbuff_t *tvb, gint *pOffset)
+ {
+- addString(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, tvb->real_data); *pOffset+=4;
++ proto_tree_add_item(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_len, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_cid, tvb, *pOffset, 16, TRUE); *pOffset+=16;
+ }
+@@ -169,7 +145,7 @@
+ proto_tree *nodeid_tree;
+ int ServiceId = 0;
+
+- addString(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, tvb->real_data); *pOffset+=4;
++ proto_tree_add_item(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_len, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_cid, tvb, *pOffset, 16, TRUE); *pOffset+=16;
+ proto_tree_add_item(tree, hf_opcua_transport_rqid, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+@@ -195,7 +171,7 @@
+
+ void parseAbort(proto_tree *tree, tvbuff_t *tvb, gint *pOffset)
+ {
+- addString(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, tvb->real_data); *pOffset+=4;
++ proto_tree_add_item(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_len, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_cid, tvb, *pOffset, 16, TRUE); *pOffset+=16;
+ proto_tree_add_item(tree, hf_opcua_transport_rqid, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+@@ -203,7 +179,7 @@
+
+ void parseError(proto_tree *tree, tvbuff_t *tvb, gint *pOffset)
+ {
+- addString(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, tvb->real_data); *pOffset+=4;
++ proto_tree_add_item(tree, hf_opcua_transport_sig, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_len, tvb, *pOffset, 4, TRUE); *pOffset+=4;
+ proto_tree_add_item(tree, hf_opcua_transport_cid, tvb, *pOffset, 16, TRUE); *pOffset+=16;
+ proto_tree_add_item(tree, hf_opcua_transport_rqid, tvb, *pOffset, 4, TRUE); *pOffset+=4;
Index: debian/patches/33_fix_afs_crash.dpatch
===================================================================
--- debian/patches/33_fix_afs_crash.dpatch (revision 0)
+++ debian/patches/33_fix_afs_crash.dpatch (revision 14468)
@@ -0,0 +1,36 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## 33_fix_afs_crash.dpatch by <balint@balintreczey.hu>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: Fix crash in AFS dissector
+
+@DPATCH@
+
+--- wireshark-1.0.2/epan/dissectors/packet-afs.c.orig 2009-09-28 13:19:22.000000000 +0200
++++ wireshark-1.0.2/epan/dissectors/packet-afs.c 2009-09-28 13:23:37.000000000 +0200
+@@ -414,19 +414,12 @@
+ /* Output a rx style string, up to a maximum length first
+ 4 bytes - length, then char data */
+ #define OUT_RXString(field) \
+- { guint32 i,len; \
+- char *tmp; \
+- const guint8 *p; \
+- i = tvb_get_ntohl(tvb, offset); \
+- offset += 4; \
+- p = tvb_get_ptr(tvb,offset,i); \
+- len = ((i+4-1)/4)*4; \
+- tmp = ep_alloc(i+1); \
+- memcpy(tmp, p, i); \
+- tmp[i] = '\0'; \
+- proto_tree_add_string(tree, field, tvb, offset-4, len+4, \
+- (void *)tmp); \
+- offset += len; \
++ { guint32 i_orxs,len_orxs; \
++ i_orxs = tvb_get_ntohl(tvb, offset); \
++ len_orxs = ((i_orxs+4-1)/4)*4 + 4; \
++ proto_tree_add_item(tree, field, tvb, offset-4, len_orxs, \
++ FALSE); \
++ offset += len_orxs; \
+ }
+
+ /* Output a fixed length vectorized string (each char is a 32 bit int) */
Index: debian/patches/30_pcnfsd_crash_fix.dpatch
===================================================================
--- debian/patches/30_pcnfsd_crash_fix.dpatch (revision 0)
+++ debian/patches/30_pcnfsd_crash_fix.dpatch (revision 14468)
@@ -0,0 +1,84 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## 24__pcnfsd_crash_fix.dpatch by <balint@balintreczey.hu>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: Fix buffer allocation to prevent crash
+
+@DPATCH@
+
+Index: trunk/epan/dissectors/packet-pcnfsd.c
+===================================================================
+--- trunk/epan/dissectors/packet-pcnfsd.c (revision 28127)
++++ trunk/epan/dissectors/packet-pcnfsd.c (revision 28128)
+@@ -211,7 +211,10 @@
+ }
+
+ if (ident) {
+- pcnfsd_decode_obscure(ident, strlen(ident));
++ /* Only attempt to decode the ident if it has been specified */
++ if (strcmp(ident, RPC_STRING_EMPTY))
++ pcnfsd_decode_obscure(ident, (int)strlen(ident));
++
+ if (ident_tree)
+ proto_tree_add_string(ident_tree,
+ hf_pcnfsd_auth_ident_clear,
+@@ -238,7 +241,10 @@
+ }
+
+ if (password) {
+- pcnfsd_decode_obscure(password, strlen(password));
++ /* Only attempt to decode the password if it has been specified */
++ if (strcmp(password, RPC_STRING_EMPTY))
++ pcnfsd_decode_obscure(password, (int)strlen(password));
++
+ if (password_tree)
+ proto_tree_add_string(password_tree,
+ hf_pcnfsd_auth_password_clear,
+Index: trunk/epan/dissectors/packet-rpc.c
+===================================================================
+--- trunk/epan/dissectors/packet-rpc.c (revision 28127)
++++ trunk/epan/dissectors/packet-rpc.c (revision 28128)
+@@ -626,24 +626,21 @@
+ char *formatted;
+
+ formatted = format_text(string_buffer, strlen(string_buffer));
+- /* alloc maximum data area */
+-#define STRING_BUFFER_PRINT_MAX_LEN (strlen(formatted)+12+1)
+- string_buffer_print = (char*)ep_alloc(STRING_BUFFER_PRINT_MAX_LEN);
+ /* copy over the data and append <TRUNCATED> */
+- g_snprintf(string_buffer_print, STRING_BUFFER_PRINT_MAX_LEN, "%s<TRUNCATED>", formatted);
++ string_buffer_print=ep_strdup_printf("%s%s", formatted, RPC_STRING_TRUNCATED);
+ } else {
+- string_buffer_print="<DATA><TRUNCATED>";
++ string_buffer_print=RPC_STRING_DATA RPC_STRING_TRUNCATED;
+ }
+ } else {
+ if (string_data) {
+ string_buffer_print =
+ ep_strdup(format_text(string_buffer, strlen(string_buffer)));
+ } else {
+- string_buffer_print="<DATA>";
++ string_buffer_print=RPC_STRING_DATA;
+ }
+ }
+ } else {
+- string_buffer_print="<EMPTY>";
++ string_buffer_print=RPC_STRING_EMPTY;
+ }
+
+ if (tree) {
+Index: trunk/epan/dissectors/packet-rpc.h
+===================================================================
+--- trunk/epan/dissectors/packet-rpc.h (revision 28127)
++++ trunk/epan/dissectors/packet-rpc.h (revision 28128)
+@@ -93,6 +93,10 @@
+ #define AUTHDES_NAMEKIND_FULLNAME 0
+ #define AUTHDES_NAMEKIND_NICKNAME 1
+
++#define RPC_STRING_EMPTY "<EMPTY>"
++#define RPC_STRING_DATA "<DATA>"
++#define RPC_STRING_TRUNCATED "<TRUNCATED>"
++
+ extern value_string rpc_authgss_svc[];
+ typedef enum {
+ FLAVOR_UNKNOWN, /* authentication flavor unknown */
Index: debian/patches/00list
===================================================================
--- debian/patches/00list (revision 13620)
+++ debian/patches/00list (revision 14468)
@@ -17,3 +17,6 @@
25_security_fixes_from_1.0.5
26_security_fixes_from_1.0.6
27_security_fixes_from_1.0.7
+30_pcnfsd_crash_fix.dpatch
+33_fix_afs_crash.dpatch
+34_fix_opcua_lockup.dpatch
Reply to: