On Sun, 23 May 2010 16:56:27 -0600, Alex Rousskov <[email protected]> wrote: > On 05/01/2010 07:49 PM, Amos Jeffries wrote: >> Alex Rousskov wrote: >>> Fixed IpAddress port printing for ports higher than 9999: >>> snprintf includes zero-terminator in its size limit, so 7 >>> rather than 6 bytes are needed to snprintf a colon followed >>> by 5 port digits. >>> >>> Whether the bug has any runtime effects in the current code, >>> I do not know, but I did waste a few hours following >>> misleading debugging output. > > >> +1. Please commit with tweak: > > Committed (r10494). Please port to v3.1 (the original patch may work). > > >> MAX_IPSTRLEN definition needs +1 as well to prevent this introducing a >> buffer overflow. > > ToURL() operates on a buffer of blen length (not necessarily > MAX_IPSTRLEN) and already checks for overflows. I do not see how it can > cause a buffer overflow even if MAX_IPSTRLEN is 0. > > If you are not worried about overflows in ToURL() but about MAX_IPSTRLEN > being too small, current MAX_IPSTRLEN=75 is probably already more than > any IP address can consume: > > IPv4: 22 (xxx.xxx.xxx.xxx:ppppp) > IPv6: 1+45+1+7=54? > (http://stackoverflow.com/questions/166132/maximum-length-of-the-textual-representation-of-an-ipv6-address) > > Did I miss something?
ToURL and NtoA should be fine, as you say they are designed to work with external buffers which could be small. I'm more worried about the external code allocating those buffers based on MAX_IPSTRLEN. Which if it was out would make them all truncate. I seem to be was miss-remembering from when MAX_IPSTRLEN was 48. 75 is okay. Another look over the ToURL and children shows ToHostname assuming the port and brackets will take 7 bytes. But that ignores the colon between port and encoded IP. Fixed that now too. Amos
