<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
Would there be any objection against replacing all strncpy instances with strlcpy? Then we won't encounter non null terminated buffers for sure.<br>
<br>
<br>
-------- Original message --------<br>
From: Michael Altizer <xiche@...1778...> <br>
Date: 08/16/2013 3:54 PM (GMT-08:00) <br>
To: snort-devel@lists.sourceforge.net <br>
Subject: Re: [Snort-devel] Possible Issues with strncpy() calls in DAQ-2.0.x and Snort-2.9.5.x
<br>
<br>
<br>
<div style="background-color:#FFFFFF">
<div class="x_moz-cite-prefix">On 08/13/2013 12:55 PM, Bill Parker wrote:<br>
</div>
<blockquote type="cite">
<div>Hello All,</div>
<div><br>
</div>
<div><span class="" style="white-space:pre"></span>In DAQ-2.0.1, directory 'os-daq-modules', file 'daq_afpacket.c',</div>
<div>I found two instances for calls to strncpy() which are not NULL</div>
<div>terminated (compared to a call to strcpy() which NULL terminates</div>
<div>the resulting string).</div>
<div><br>
</div>
<div>The patch file below adds the NULL byte:</div>
<div><br>
</div>
<div>--- daq_afpacket.c.orig 2013-08-12 19:07:36.190972370 -0700</div>
<div>+++ daq_afpacket.c      2013-08-12 19:10:11.983969620 -0700</div>
<div>@@ -110,6 +110,7 @@</div>
<div> </div>
<div>     memset(&ifr, 0, sizeof(ifr));</div>
<div>     strncpy(ifr.ifr_name, device, sizeof(ifr.ifr_name));</div>
<div>+    ifr.ifr_name[sizeof(ifr.ifr_name)-1] = '\0';</div>
<div> </div>
<div>     if (ioctl(instance->fd, SIOCGIFINDEX, &ifr) == -1)</div>
<div>         return -1;</div>
<div>@@ -151,6 +152,7 @@</div>
<div> </div>
<div>     memset(&ifr, 0, sizeof(ifr));</div>
<div>     strncpy(ifr.ifr_name, instance->name, sizeof(ifr.ifr_name));</div>
<div>+    ifr.ifr_name[sizeof(ifr.ifr_name)-1] = '\0';</div>
<div> </div>
<div>     if (ioctl(instance->fd, SIOCGIFHWADDR, &ifr) == -1)</div>
<div>     {</div>
<div><span class="" style="white-space:pre"></span> </div>
<div>A 'make' and 'make install' result in successful compilation :)</div>
</blockquote>
<br>
Thanks, Bill, but the name that it is copying from will always be a NULL-terminated string of at most IFNAMSIZ bytes (including the NULL), so it shouldn't really be an issue.  See afpacket_daq_initialize() for the sanity checking on the device names.<br>
</div>
</body>
</html>