unsafe strlcpy

[Maxime Villard <max%m00nbsd.net@localhost>]

[I am not subscribed to this list, so if you want to answer, make sure to CC me]

Contrary to what it seems, strlcpy is not safe with an untrusted second
argument, because it does strlen on it, and therefore expects it to be NUL-
terminated.

It looks like we have some problems in some syscalls, which call strlcpy on
user buffers that got copyin'ed. If these buffers aren't NUL-terminated,
strlcpy will read memory until encountering a '\0'.

Example, netbsd32_ioctl.c with kASan:

	#include <stdio.h>
	#include <stdlib.h>
	#include <unistd.h>
	#include <sys/ioctl.h>
	#include <fcntl.h>
	#include <net/if.h>
	#include <sys/sockio.h>
	#include <string.h>
	int main() {
		char buf[256];
		memset(buf, 0xAA, sizeof(buf));
		int fd = open("test.c", O_RDONLY);
		ioctl(fd, SIOCGIFADDRPREF, buf);
	}

	$ gcc -o test test.c -m32
	$ ./test
	ASan: Unauthorized Access In 0x...: Addr 0x... [1 byte, read, RedZonePartial]
	#0 0x... in strlcpy <netbsd>
	#1 0x... in netbsd32_ioctl <netbsd>
	#2 0x... in netbsd32_syscall <netbsd>
	#3 0x... in handle_syscall <netbsd>

What's worse, in if_pppoe.c, it looks like we're calling strlcpy on a packet,
with no guarantee that the string in the packet is NUL-terminated. This could
trigger remote DoS.

I think that strlcpy has a bad design and should be replaced by the safer
copystr.

In PPPoE I think we should drop the string stuff, calling printf is already a
bad idea anyway.

Maxime