Hi Jeremy,
On 03/29/2012 11:49 AM, Jeremy Allison wrote:
The branch, master has been updated
via 5df1c11 Start to add truncate checks on all uses of strlcpy().
Reading lwn has it's uses :-)
This patch broke the internal DNS server if at least one interface has
a local-link address (that is to say if enable IPv6).
Furthermore if bind interfaces only = yes and interfaces = xxx are
specified in the configuration then all samba 4 services are broken.
diff --git a/lib/util/util_net.c b/lib/util/util_net.c
index 637c52b..69e5324 100644
--- a/lib/util/util_net.c
+++ b/lib/util/util_net.c
@@ -107,9 +107,11 @@ static bool interpret_string_addr_pref(struct
sockaddr_storage *pss,
*/
if (p&& (p> str)&& ((scope_id = if_nametoindex(p+1)) != 0)) {
- strlcpy(addr, str,
- MIN(PTR_DIFF(p,str)+1,
- sizeof(addr)));
+ size_t len = MIN(PTR_DIFF(p,str)+1, sizeof(addr));
+ if (strlcpy(addr, str, len)>= len) {
+ /* Truncate. */
+ return false;
+ }
str = addr;
}
}
@@ -332,9 +334,11 @@ bool is_ipaddress_v6(const char *str)
*/
if (p&& (p> str)&& (if_nametoindex(p+1) != 0)) {
- strlcpy(addr, str,
- MIN(PTR_DIFF(p,str)+1,
- sizeof(addr)));
+ size_t len = MIN(PTR_DIFF(p,str)+1, sizeof(addr));
+ if (strlcpy(addr, str, len)>= len) {
+ /* Truncate. */
+ return false;
+ }
sp = addr;
}
ret = inet_pton(AF_INET6, sp,&dest6);
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.
@@ -723,7 +727,10 @@ static const char *get_socket_addr(int fd, char *addr_buf,
size_t addr_len)
* zero IPv6 address. No good choice here.
*/
- strlcpy(addr_buf, "0.0.0.0", addr_len);
+ if (strlcpy(addr_buf, "0.0.0.0", addr_len)>= addr_len) {
+ /* Truncate ! */
+ return NULL;
+ }
Here it seems also wrong if you have an address like 1.2.3.4 as addr_len
will be the same as strlen("0.0.0.0"), didn't had a closer look.
I just focused on the IPv6 issues, maybe we should revert your patch
while you had a closer look on the different use cases for strlcpy and
then push a valid change ?
Matthieu.
--
Matthieu Patou
Samba Team
http://samba.org
