On Thu, Aug 27, 2015 at 09:44:33AM -0600, Todd C. Miller wrote:
> On Thu, 27 Aug 2015 15:47:18 +0200, Alexander Bluhm wrote:
>
> > When syslogd is reloading a modified config, it does a reexec on
> > itself. For this it uses the original arguments of main(). The
> > function loghost_parse() modifies the optarg memory it is operating
> > on. To prevent that the exec arguments have been tampered, pass a
> > copy of optarg to loghost_parse().
>
> If you are going to do it this way you really need to check the
> strlcpy() return value and error out on overflow.
Yes, that is better.
ok?
bluhm
Index: usr.sbin/syslogd/syslogd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.178
diff -u -p -r1.178 syslogd.c
--- usr.sbin/syslogd/syslogd.c 25 Aug 2015 17:14:16 -0000 1.178
+++ usr.sbin/syslogd/syslogd.c 27 Aug 2015 15:50:29 -0000
@@ -346,6 +346,8 @@ main(int argc, char *argv[])
struct timeval to;
const char *errstr;
char *p;
+ char udparg[NI_MAXHOST+2+NI_MAXSERV];
+ char tcparg[NI_MAXHOST+2+NI_MAXSERV];
int ch, i;
int lockpipe[2] = { -1, -1}, pair[2], nullfd, fd;
@@ -393,12 +395,18 @@ main(int argc, char *argv[])
path_ctlsock = optarg;
break;
case 'T': /* allow tcp and listen on address */
- if (loghost_parse(optarg, NULL, &listen_host,
+ if (strlcpy(tcparg, optarg, sizeof(tcparg)) >=
+ sizeof(tcparg))
+ errx(1, "listen address too long: %s", optarg);
+ if (loghost_parse(tcparg, NULL, &listen_host,
&listen_port) == -1)
errx(1, "bad listen address: %s", optarg);
break;
case 'U': /* allow udp only from address */
- if (loghost_parse(optarg, NULL, &bind_host, &bind_port)
+ if (strlcpy(udparg, optarg, sizeof(udparg)) >=
+ sizeof(udparg))
+ errx(1, "bind address too long: %s", optarg);
+ if (loghost_parse(udparg, NULL, &bind_host, &bind_port)
== -1)
errx(1, "bad bind address: %s", optarg);
break;
