Wireshark · Wireshark-dev: Re: [Wireshark-dev] address_to_str_buf not checking available buffer length

[Andrej Mikus <wireshark-dev@xxxxxxxx>]

On Mon, 29.Jan.07 12:49:00 -0800, Gerald Combs wrote:
> Andrej Mikus wrote:
> 
> > In file to_str.c I notice function address_to_str_buf that takes buf_len
> > as argument, but does not use it for IPv4 and IPv6 addresses:
> > 
> Unfortunately, there wasn't a mechanism to ensure that the buffer was
> big enough.  I've checked in a change (r20607) that should fix this.  It
> looks like this could potentially affect the ISUP statistics, so I'll
> add it to the advisory list for 0.99.5.

I was planning to propose a patch already (attached), so this allowed me
to see how Subversion handles conflicts :-)

IMO, the function should return an error string instead of cowardly
bailing out with an empty string.

inet_ntop does the latter by default, so it does not need any additional
check, just passing buf_len instead of INET6_ADDRSTRLEN.

At the other side I like the check you made into ip_to_str_buf.

My proposal would be to combine the patches and use MAX_IP_STR_LEN,
and approach below for both IPv4 and IPv6 in address_to_str_buf.

   case AT_IPv6:
+<<<<<<< .mine
+    if ( inet_ntop(AF_INET6, addr->data, buf, buf_len) == NULL ) /* Returns NULL if no space and does not touch buf */
+       g_snprintf ( buf, buf_len, "@@@@@@@@" );                 /* Let the unexpected value alert user */

If accepted, I am ready to supply a patch on top of current rev .r20649
Let me know pls.

Andrej

Attachment: to_str.patch.gz
Description: application/gunzip