wild use of strlcpy() broke ipv6 support

Jeremy Allison jra at samba.org
Fri Mar 30 22:21:29 MDT 2012


On Fri, Mar 30, 2012 at 08:53:21PM -0700, Jeremy Allison wrote:
> On Fri, Mar 30, 2012 at 08:52:09PM -0700, Jeremy Allison wrote:
> > On Fri, Mar 30, 2012 at 06:02:06PM -0700, Matthieu Patou wrote:
> > > At least for this two changes you didn't get completely the sense of
> > > the strlcpy(), the idea is that if you have
> > > fe80::221:ccff:fe5f:7e51%eth0 to get the number of the interface and
> > > remove what is after '%'.
> > > so we should in this case always truncate and ihmo it's not a problem.
> > 
> > Ok - I'm boilerplate fixing all strlcpy uses. If it truncates
> > then by definition it's a problem, and we shouldn't be using
> > strlcpy() - we should be correctly paying attention to
> > the length we really want.
> > 
> > I do (vaguely :-) remember writing this code (or something
> > like it). I'll take a look and fix up asap.
> 
> Ah yes - now I looked at it I did write this code originally :-).
> 
> I'll fix.

Here's the fix (attached). autobuilding now.

Sorry for the breakage, but we really do need to
correctly use strlcpy/strlcat to check if we ever
get truncation (I'm at fault here, as I introduced
them originally - and added the incorrect idiom
when using them).

I'll try and be a little more careful when adding
the checks if we're doing anything sophisticated
with the lengths. It's a very rare case where
we're using truncation as a desired side effect
(and is an *explicitly* incorrect use of the API :-).

Thanks for finding the problem !

Jeremy.
-------------- next part --------------
diff --git a/lib/util/util_net.c b/lib/util/util_net.c
index 69e5324..36b3fcb 100644
--- a/lib/util/util_net.c
+++ b/lib/util/util_net.c
@@ -107,11 +107,18 @@ static bool interpret_string_addr_pref(struct sockaddr_storage *pss,
 		 */
 
 		if (p && (p > str) && ((scope_id = if_nametoindex(p+1)) != 0)) {
-			size_t len = MIN(PTR_DIFF(p,str)+1, sizeof(addr));
-			if (strlcpy(addr, str, len) >= len) {
-				/* Truncate. */
+			/* Length of string we want to copy.
+			   This is IP:v6:addr (removing the %ifname).
+			 */
+			size_t len = PTR_DIFF(p,str);
+
+			if (len+1 > sizeof(addr)) {
+				/* string+nul too long for array. */
 				return false;
 			}
+			memcpy(addr, str, len);
+			addr[len] = '\0';
+
 			str = addr;
 		}
 	}


More information about the samba-technical mailing list