diff options
author | Reyk Floeter <reyk@cvs.openbsd.org> | 2007-02-08 13:32:25 +0000 |
---|---|---|
committer | Reyk Floeter <reyk@cvs.openbsd.org> | 2007-02-08 13:32:25 +0000 |
commit | 0ae251368e20313e60a071549d04cb246ecf9d0a (patch) | |
tree | 9a87bef487bdb3c936993cb48cd01ef6a23251c3 /usr.sbin | |
parent | 352d740dd85eb470c920da7405b235e0849a6c80 (diff) |
carefully check some return values and make lint happier. never pass
any truncated strings (table names/anchors/tags/...) to pf and the
kernel.
ok pyr@
Diffstat (limited to 'usr.sbin')
-rw-r--r-- | usr.sbin/hoststated/check_tcp.c | 7 | ||||
-rw-r--r-- | usr.sbin/hoststated/control.c | 16 | ||||
-rw-r--r-- | usr.sbin/hoststated/hoststated.c | 8 | ||||
-rw-r--r-- | usr.sbin/hoststated/parse.y | 15 | ||||
-rw-r--r-- | usr.sbin/hoststated/pfe.c | 6 | ||||
-rw-r--r-- | usr.sbin/hoststated/pfe_filter.c | 138 | ||||
-rw-r--r-- | usr.sbin/hoststated/ssl.c | 6 | ||||
-rw-r--r-- | usr.sbin/relayd/check_tcp.c | 7 | ||||
-rw-r--r-- | usr.sbin/relayd/control.c | 16 | ||||
-rw-r--r-- | usr.sbin/relayd/parse.y | 15 | ||||
-rw-r--r-- | usr.sbin/relayd/pfe.c | 6 | ||||
-rw-r--r-- | usr.sbin/relayd/pfe_filter.c | 138 | ||||
-rw-r--r-- | usr.sbin/relayd/relayd.c | 8 | ||||
-rw-r--r-- | usr.sbin/relayd/ssl.c | 6 |
14 files changed, 268 insertions, 124 deletions
diff --git a/usr.sbin/hoststated/check_tcp.c b/usr.sbin/hoststated/check_tcp.c index 158800305eb..905b0becbfd 100644 --- a/usr.sbin/hoststated/check_tcp.c +++ b/usr.sbin/hoststated/check_tcp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: check_tcp.c,v 1.21 2007/02/07 15:13:00 reyk Exp $ */ +/* $OpenBSD: check_tcp.c,v 1.22 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -247,7 +247,8 @@ tcp_read_buf(int s, short event, void *arg) "tcp_read_buf: check failed"); return; default: - buf_add(cte->buf, rbuf, br); + if (buf_add(cte->buf, rbuf, br) == -1) + fatal("tcp_read_buf: buf_add error"); if (cte->validate_read != NULL) { if (cte->validate_read(cte) != 0) goto retry; @@ -326,7 +327,7 @@ check_http_code(struct ctl_tcp_event *cte) host->up = HOST_DOWN; return (1); } - strlcpy(scode, head, sizeof(scode)); + (void)strlcpy(scode, head, sizeof(scode)); code = strtonum(scode, 100, 999, &estr); if (estr != NULL) { log_debug("check_http_code: %s failed " diff --git a/usr.sbin/hoststated/control.c b/usr.sbin/hoststated/control.c index 20e1cc04ff5..bfd318f45ea 100644 --- a/usr.sbin/hoststated/control.c +++ b/usr.sbin/hoststated/control.c @@ -1,4 +1,4 @@ -/* $OpenBSD: control.c,v 1.12 2007/02/07 13:39:58 reyk Exp $ */ +/* $OpenBSD: control.c,v 1.13 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org> @@ -56,7 +56,12 @@ control_init(void) bzero(&sun, sizeof(sun)); sun.sun_family = AF_UNIX; - strlcpy(sun.sun_path, HOSTSTATED_SOCKET, sizeof(sun.sun_path)); + if (strlcpy(sun.sun_path, HOSTSTATED_SOCKET, + sizeof(sun.sun_path)) >= sizeof(sun.sun_path)) { + log_warn("control_init: %s name too long", HOSTSTATED_SOCKET); + close(fd); + return (-1); + } if (unlink(HOSTSTATED_SOCKET) == -1) if (errno != ENOENT) { @@ -69,10 +74,10 @@ control_init(void) if (bind(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1) { log_warn("control_init: bind: %s", HOSTSTATED_SOCKET); close(fd); - umask(old_umask); + (void)umask(old_umask); return (-1); } - umask(old_umask); + (void)umask(old_umask); if (chmod(HOSTSTATED_SOCKET, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) == -1) { log_warn("control_init: chmod"); @@ -106,8 +111,7 @@ control_listen(void) void control_cleanup(void) { - - unlink(HOSTSTATED_SOCKET); + (void)unlink(HOSTSTATED_SOCKET); } /* ARGSUSED */ diff --git a/usr.sbin/hoststated/hoststated.c b/usr.sbin/hoststated/hoststated.c index 32aba7bfc10..8e9dd49073d 100644 --- a/usr.sbin/hoststated/hoststated.c +++ b/usr.sbin/hoststated/hoststated.c @@ -1,4 +1,4 @@ -/* $OpenBSD: hoststated.c,v 1.15 2007/02/07 13:30:17 reyk Exp $ */ +/* $OpenBSD: hoststated.c,v 1.16 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -152,8 +152,10 @@ main(int argc, char *argv[]) if (getpwnam(HOSTSTATED_USER) == NULL) errx(1, "unknown user %s", HOSTSTATED_USER); - if (!debug) - daemon(1, 0); + if (!debug) { + if (daemon(1, 0) == -1) + err(1, "failed to daemonize"); + } log_info("startup"); diff --git a/usr.sbin/hoststated/parse.y b/usr.sbin/hoststated/parse.y index fe81c4569aa..c5d53b44409 100644 --- a/usr.sbin/hoststated/parse.y +++ b/usr.sbin/hoststated/parse.y @@ -1,4 +1,4 @@ -/* $OpenBSD: parse.y,v 1.23 2007/02/07 15:17:46 reyk Exp $ */ +/* $OpenBSD: parse.y,v 1.24 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -412,8 +412,9 @@ tableoptsl : host { } table->check = CHECK_HTTP_CODE; table->retcode = $5; - asprintf(&table->sendbuf, "HEAD %s HTTP/1.0\r\n\r\n", - $3); + if (asprintf(&table->sendbuf, + "HEAD %s HTTP/1.0\r\n\r\n", $3) == -1) + fatal("asprintf"); free($3); if (table->sendbuf == NULL) fatal("out of memory"); @@ -424,8 +425,9 @@ tableoptsl : host { table->flags |= F_SSL; } table->check = CHECK_HTTP_DIGEST; - asprintf(&table->sendbuf, "GET %s HTTP/1.0\r\n\r\n", - $3); + if (asprintf(&table->sendbuf, + "GET %s HTTP/1.0\r\n\r\n", $3) == -1) + fatal("asprintf"); free($3); if (table->sendbuf == NULL) fatal("out of memory"); @@ -913,7 +915,8 @@ cmdline_symset(char *s) if ((sym = malloc(len)) == NULL) errx(1, "cmdline_symset: malloc"); - strlcpy(sym, s, len); + if (strlcpy(sym, s, len) >= len) + errx(1, "cmdline_symset: macro too long"); ret = symset(sym, val + 1, 1); free(sym); diff --git a/usr.sbin/hoststated/pfe.c b/usr.sbin/hoststated/pfe.c index d02fbce1251..0af74099611 100644 --- a/usr.sbin/hoststated/pfe.c +++ b/usr.sbin/hoststated/pfe.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfe.c,v 1.13 2007/02/06 11:21:35 pyr Exp $ */ +/* $OpenBSD: pfe.c,v 1.14 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -132,7 +132,9 @@ pfe(struct hoststated *x_env, int pipe_parent2pfe[2], int pipe_parent2hce[2], event_add(&ibuf_main->ev, NULL); TAILQ_INIT(&ctl_conns); - control_listen(); + + if (control_listen() == -1) + fatalx("pfe: control socket listen failed"); event_dispatch(); pfe_shutdown(); diff --git a/usr.sbin/hoststated/pfe_filter.c b/usr.sbin/hoststated/pfe_filter.c index 060024f1219..6a38260d606 100644 --- a/usr.sbin/hoststated/pfe_filter.c +++ b/usr.sbin/hoststated/pfe_filter.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfe_filter.c,v 1.11 2007/02/07 14:45:12 reyk Exp $ */ +/* $OpenBSD: pfe_filter.c,v 1.12 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -79,12 +79,16 @@ init_tables(struct hoststated *env) i = 0; TAILQ_FOREACH(service, &env->services, entry) { - (void)strlcpy(tables[i].pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(tables[i].pfrt_anchor)); - (void)strlcat(tables[i].pfrt_anchor, service->name, - sizeof(tables[i].pfrt_anchor)); - (void)strlcpy(tables[i].pfrt_name, service->name, - sizeof(tables[i].pfrt_name)); + if (strlcpy(tables[i].pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(tables[i].pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(tables[i].pfrt_anchor, service->name, + sizeof(tables[i].pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcpy(tables[i].pfrt_name, service->name, + sizeof(tables[i].pfrt_name)) >= + sizeof(tables[i].pfrt_name)) + goto toolong; tables[i].pfrt_flags |= PFR_TFLAG_PERSIST; i++; } @@ -110,6 +114,11 @@ init_tables(struct hoststated *env) */ TAILQ_FOREACH(service, &env->services, entry) flush_table(env, service); + + return; + + toolong: + fatal("init_tables: name too long"); } void @@ -119,14 +128,20 @@ kill_tables(struct hoststated *env) { memset(&io, 0, sizeof(io)); TAILQ_FOREACH(service, &env->services, entry) { - (void)strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcat(io.pfrio_table.pfrt_anchor, service->name, - sizeof(io.pfrio_table.pfrt_anchor)); + if (strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(io.pfrio_table.pfrt_anchor, service->name, + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; if (ioctl(env->pf->dev, DIOCRCLRTABLES, &io) == -1) fatal("kill_tables: ioctl faile: ioctl failed"); } log_debug("kill_tables: deleted %d tables", io.pfrio_ndel); + return; + + toolong: + fatal("kill_tables: name too long"); } void @@ -155,12 +170,16 @@ sync_table(struct hoststated *env, struct service *service, struct table *table) io.pfrio_size = table->up; io.pfrio_size2 = 0; io.pfrio_buffer = addlist; - (void)strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcat(io.pfrio_table.pfrt_anchor, service->name, - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcpy(io.pfrio_table.pfrt_name, service->name, - sizeof(io.pfrio_table.pfrt_name)); + if (strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(io.pfrio_table.pfrt_anchor, service->name, + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcpy(io.pfrio_table.pfrt_name, service->name, + sizeof(io.pfrio_table.pfrt_name)) >= + sizeof(io.pfrio_table.pfrt_name)) + goto toolong; i = 0; TAILQ_FOREACH(host, &table->hosts, entry) { @@ -199,6 +218,10 @@ sync_table(struct hoststated *env, struct service *service, struct table *table) log_debug("sync_table: table %s: %d added, %d deleted, %d changed", io.pfrio_table.pfrt_name, io.pfrio_nadd, io.pfrio_ndel, io.pfrio_nchange); + return; + + toolong: + fatal("sync_table: name too long"); } void @@ -207,16 +230,23 @@ flush_table(struct hoststated *env, struct service *service) struct pfioc_table io; memset(&io, 0, sizeof(io)); - (void)strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcat(io.pfrio_table.pfrt_anchor, service->name, - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcpy(io.pfrio_table.pfrt_name, service->name, - sizeof(io.pfrio_table.pfrt_name)); + if (strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(io.pfrio_table.pfrt_anchor, service->name, + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcpy(io.pfrio_table.pfrt_name, service->name, + sizeof(io.pfrio_table.pfrt_name)) >= + sizeof(io.pfrio_table.pfrt_name)) + goto toolong; if (ioctl(env->pf->dev, DIOCRCLRADDRS, &io) == -1) fatal("flush_table: cannot flush table"); log_debug("flush_table: flushed table %s", service->name); return; + + toolong: + fatal("flush_table: name too long"); } int @@ -227,7 +257,7 @@ transaction_init(struct hoststated *env, const char *anchor) env->pf->pft.array = &env->pf->pfte; memset(&env->pf->pfte, 0, sizeof env->pf->pfte); - strlcpy(env->pf->pfte.anchor, anchor, PF_ANCHOR_NAME_SIZE); + (void)strlcpy(env->pf->pfte.anchor, anchor, PF_ANCHOR_NAME_SIZE); env->pf->pfte.rs_num = PF_RULESET_RDR; if (ioctl(env->pf->dev, DIOCXBEGIN, &env->pf->pft) == -1) @@ -254,13 +284,23 @@ sync_ruleset(struct hoststated *env, struct service *service, int enable) char anchor[PF_ANCHOR_NAME_SIZE]; bzero(anchor, sizeof(anchor)); - (void)strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)); - (void)strlcat(anchor, service->name, sizeof(anchor)); - transaction_init(env, anchor); + if (strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(anchor, service->name, sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (transaction_init(env, anchor) == -1) { + log_warn("sync_ruleset: transaction init failed"); + return; + } if (!enable) { - transaction_commit(env); - log_debug("sync_ruleset: rules removed"); + if (transaction_commit(env) == -1) + log_warn("sync_ruleset: " + "remove rules transaction failed"); + else + log_debug("sync_ruleset: rules removed"); return; } @@ -306,8 +346,9 @@ sync_ruleset(struct hoststated *env, struct service *service, int enable) } pio.addr.addr.type = PF_ADDR_TABLE; - (void)strlcpy(pio.addr.addr.v.tblname, service->name, - sizeof(pio.addr.addr.v.tblname)); + if (strlcpy(pio.addr.addr.v.tblname, service->name, + sizeof(pio.addr.addr.v.tblname)) >= sizeof(pio.addr.addr.v.tblname)) + fatal("sync_ruleset: table name too long"); if (ioctl(env->pf->dev, DIOCADDADDR, &pio) == -1) fatal("sync_ruleset: cannot add address to pool"); @@ -321,7 +362,12 @@ sync_ruleset(struct hoststated *env, struct service *service, int enable) fatal("cannot add rule"); log_debug("sync_ruleset: rule added"); } - transaction_commit(env); + if (transaction_commit(env) == -1) + log_warn("sync_ruleset: add rules transaction failed"); + return; + + toolong: + fatal("sync_ruleset: name too long"); } void @@ -332,13 +378,27 @@ flush_rulesets(struct hoststated *env) kill_tables(env); TAILQ_FOREACH(service, &env->services, entry) { - strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)); - strlcat(anchor, service->name, sizeof(anchor)); - transaction_init(env, anchor); - transaction_commit(env); + if (strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(anchor, service->name, sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (transaction_init(env, anchor) == -1 || + transaction_commit(env) == -1) + log_warn("flush_rulesets: transaction for %s/ failed", + HOSTSTATED_ANCHOR); } - strlcpy(anchor, HOSTSTATED_ANCHOR, sizeof(anchor)); - transaction_init(env, anchor); - transaction_commit(env); + if (strlcpy(anchor, HOSTSTATED_ANCHOR, sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (transaction_init(env, anchor) == -1 || + transaction_commit(env) == -1) + log_warn("flush_rulesets: transaction for %s failed", + HOSTSTATED_ANCHOR); log_debug("flush_rulesets: flushed rules"); + return; + + toolong: + fatal("flush_rulesets: name too long"); } diff --git a/usr.sbin/hoststated/ssl.c b/usr.sbin/hoststated/ssl.c index 87c887d5e21..ddac5c5974c 100644 --- a/usr.sbin/hoststated/ssl.c +++ b/usr.sbin/hoststated/ssl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl.c,v 1.5 2007/02/07 14:39:45 reyk Exp $ */ +/* $OpenBSD: ssl.c,v 1.6 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -94,8 +94,8 @@ ssl_read(int s, short event, void *arg) } return; } - buf_add(cte->buf, rbuf, ret); - + if (buf_add(cte->buf, rbuf, ret) == -1) + fatal("ssl_read: buf_add error"); if (cte->validate_read != NULL) { if (cte->validate_read(cte) != 0) goto retry; diff --git a/usr.sbin/relayd/check_tcp.c b/usr.sbin/relayd/check_tcp.c index 158800305eb..905b0becbfd 100644 --- a/usr.sbin/relayd/check_tcp.c +++ b/usr.sbin/relayd/check_tcp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: check_tcp.c,v 1.21 2007/02/07 15:13:00 reyk Exp $ */ +/* $OpenBSD: check_tcp.c,v 1.22 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -247,7 +247,8 @@ tcp_read_buf(int s, short event, void *arg) "tcp_read_buf: check failed"); return; default: - buf_add(cte->buf, rbuf, br); + if (buf_add(cte->buf, rbuf, br) == -1) + fatal("tcp_read_buf: buf_add error"); if (cte->validate_read != NULL) { if (cte->validate_read(cte) != 0) goto retry; @@ -326,7 +327,7 @@ check_http_code(struct ctl_tcp_event *cte) host->up = HOST_DOWN; return (1); } - strlcpy(scode, head, sizeof(scode)); + (void)strlcpy(scode, head, sizeof(scode)); code = strtonum(scode, 100, 999, &estr); if (estr != NULL) { log_debug("check_http_code: %s failed " diff --git a/usr.sbin/relayd/control.c b/usr.sbin/relayd/control.c index 20e1cc04ff5..bfd318f45ea 100644 --- a/usr.sbin/relayd/control.c +++ b/usr.sbin/relayd/control.c @@ -1,4 +1,4 @@ -/* $OpenBSD: control.c,v 1.12 2007/02/07 13:39:58 reyk Exp $ */ +/* $OpenBSD: control.c,v 1.13 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org> @@ -56,7 +56,12 @@ control_init(void) bzero(&sun, sizeof(sun)); sun.sun_family = AF_UNIX; - strlcpy(sun.sun_path, HOSTSTATED_SOCKET, sizeof(sun.sun_path)); + if (strlcpy(sun.sun_path, HOSTSTATED_SOCKET, + sizeof(sun.sun_path)) >= sizeof(sun.sun_path)) { + log_warn("control_init: %s name too long", HOSTSTATED_SOCKET); + close(fd); + return (-1); + } if (unlink(HOSTSTATED_SOCKET) == -1) if (errno != ENOENT) { @@ -69,10 +74,10 @@ control_init(void) if (bind(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1) { log_warn("control_init: bind: %s", HOSTSTATED_SOCKET); close(fd); - umask(old_umask); + (void)umask(old_umask); return (-1); } - umask(old_umask); + (void)umask(old_umask); if (chmod(HOSTSTATED_SOCKET, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP) == -1) { log_warn("control_init: chmod"); @@ -106,8 +111,7 @@ control_listen(void) void control_cleanup(void) { - - unlink(HOSTSTATED_SOCKET); + (void)unlink(HOSTSTATED_SOCKET); } /* ARGSUSED */ diff --git a/usr.sbin/relayd/parse.y b/usr.sbin/relayd/parse.y index fe81c4569aa..c5d53b44409 100644 --- a/usr.sbin/relayd/parse.y +++ b/usr.sbin/relayd/parse.y @@ -1,4 +1,4 @@ -/* $OpenBSD: parse.y,v 1.23 2007/02/07 15:17:46 reyk Exp $ */ +/* $OpenBSD: parse.y,v 1.24 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -412,8 +412,9 @@ tableoptsl : host { } table->check = CHECK_HTTP_CODE; table->retcode = $5; - asprintf(&table->sendbuf, "HEAD %s HTTP/1.0\r\n\r\n", - $3); + if (asprintf(&table->sendbuf, + "HEAD %s HTTP/1.0\r\n\r\n", $3) == -1) + fatal("asprintf"); free($3); if (table->sendbuf == NULL) fatal("out of memory"); @@ -424,8 +425,9 @@ tableoptsl : host { table->flags |= F_SSL; } table->check = CHECK_HTTP_DIGEST; - asprintf(&table->sendbuf, "GET %s HTTP/1.0\r\n\r\n", - $3); + if (asprintf(&table->sendbuf, + "GET %s HTTP/1.0\r\n\r\n", $3) == -1) + fatal("asprintf"); free($3); if (table->sendbuf == NULL) fatal("out of memory"); @@ -913,7 +915,8 @@ cmdline_symset(char *s) if ((sym = malloc(len)) == NULL) errx(1, "cmdline_symset: malloc"); - strlcpy(sym, s, len); + if (strlcpy(sym, s, len) >= len) + errx(1, "cmdline_symset: macro too long"); ret = symset(sym, val + 1, 1); free(sym); diff --git a/usr.sbin/relayd/pfe.c b/usr.sbin/relayd/pfe.c index d02fbce1251..0af74099611 100644 --- a/usr.sbin/relayd/pfe.c +++ b/usr.sbin/relayd/pfe.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfe.c,v 1.13 2007/02/06 11:21:35 pyr Exp $ */ +/* $OpenBSD: pfe.c,v 1.14 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -132,7 +132,9 @@ pfe(struct hoststated *x_env, int pipe_parent2pfe[2], int pipe_parent2hce[2], event_add(&ibuf_main->ev, NULL); TAILQ_INIT(&ctl_conns); - control_listen(); + + if (control_listen() == -1) + fatalx("pfe: control socket listen failed"); event_dispatch(); pfe_shutdown(); diff --git a/usr.sbin/relayd/pfe_filter.c b/usr.sbin/relayd/pfe_filter.c index 060024f1219..6a38260d606 100644 --- a/usr.sbin/relayd/pfe_filter.c +++ b/usr.sbin/relayd/pfe_filter.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfe_filter.c,v 1.11 2007/02/07 14:45:12 reyk Exp $ */ +/* $OpenBSD: pfe_filter.c,v 1.12 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -79,12 +79,16 @@ init_tables(struct hoststated *env) i = 0; TAILQ_FOREACH(service, &env->services, entry) { - (void)strlcpy(tables[i].pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(tables[i].pfrt_anchor)); - (void)strlcat(tables[i].pfrt_anchor, service->name, - sizeof(tables[i].pfrt_anchor)); - (void)strlcpy(tables[i].pfrt_name, service->name, - sizeof(tables[i].pfrt_name)); + if (strlcpy(tables[i].pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(tables[i].pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(tables[i].pfrt_anchor, service->name, + sizeof(tables[i].pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcpy(tables[i].pfrt_name, service->name, + sizeof(tables[i].pfrt_name)) >= + sizeof(tables[i].pfrt_name)) + goto toolong; tables[i].pfrt_flags |= PFR_TFLAG_PERSIST; i++; } @@ -110,6 +114,11 @@ init_tables(struct hoststated *env) */ TAILQ_FOREACH(service, &env->services, entry) flush_table(env, service); + + return; + + toolong: + fatal("init_tables: name too long"); } void @@ -119,14 +128,20 @@ kill_tables(struct hoststated *env) { memset(&io, 0, sizeof(io)); TAILQ_FOREACH(service, &env->services, entry) { - (void)strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcat(io.pfrio_table.pfrt_anchor, service->name, - sizeof(io.pfrio_table.pfrt_anchor)); + if (strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(io.pfrio_table.pfrt_anchor, service->name, + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; if (ioctl(env->pf->dev, DIOCRCLRTABLES, &io) == -1) fatal("kill_tables: ioctl faile: ioctl failed"); } log_debug("kill_tables: deleted %d tables", io.pfrio_ndel); + return; + + toolong: + fatal("kill_tables: name too long"); } void @@ -155,12 +170,16 @@ sync_table(struct hoststated *env, struct service *service, struct table *table) io.pfrio_size = table->up; io.pfrio_size2 = 0; io.pfrio_buffer = addlist; - (void)strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcat(io.pfrio_table.pfrt_anchor, service->name, - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcpy(io.pfrio_table.pfrt_name, service->name, - sizeof(io.pfrio_table.pfrt_name)); + if (strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(io.pfrio_table.pfrt_anchor, service->name, + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcpy(io.pfrio_table.pfrt_name, service->name, + sizeof(io.pfrio_table.pfrt_name)) >= + sizeof(io.pfrio_table.pfrt_name)) + goto toolong; i = 0; TAILQ_FOREACH(host, &table->hosts, entry) { @@ -199,6 +218,10 @@ sync_table(struct hoststated *env, struct service *service, struct table *table) log_debug("sync_table: table %s: %d added, %d deleted, %d changed", io.pfrio_table.pfrt_name, io.pfrio_nadd, io.pfrio_ndel, io.pfrio_nchange); + return; + + toolong: + fatal("sync_table: name too long"); } void @@ -207,16 +230,23 @@ flush_table(struct hoststated *env, struct service *service) struct pfioc_table io; memset(&io, 0, sizeof(io)); - (void)strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcat(io.pfrio_table.pfrt_anchor, service->name, - sizeof(io.pfrio_table.pfrt_anchor)); - (void)strlcpy(io.pfrio_table.pfrt_name, service->name, - sizeof(io.pfrio_table.pfrt_name)); + if (strlcpy(io.pfrio_table.pfrt_anchor, HOSTSTATED_ANCHOR "/", + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(io.pfrio_table.pfrt_anchor, service->name, + sizeof(io.pfrio_table.pfrt_anchor)) >= PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcpy(io.pfrio_table.pfrt_name, service->name, + sizeof(io.pfrio_table.pfrt_name)) >= + sizeof(io.pfrio_table.pfrt_name)) + goto toolong; if (ioctl(env->pf->dev, DIOCRCLRADDRS, &io) == -1) fatal("flush_table: cannot flush table"); log_debug("flush_table: flushed table %s", service->name); return; + + toolong: + fatal("flush_table: name too long"); } int @@ -227,7 +257,7 @@ transaction_init(struct hoststated *env, const char *anchor) env->pf->pft.array = &env->pf->pfte; memset(&env->pf->pfte, 0, sizeof env->pf->pfte); - strlcpy(env->pf->pfte.anchor, anchor, PF_ANCHOR_NAME_SIZE); + (void)strlcpy(env->pf->pfte.anchor, anchor, PF_ANCHOR_NAME_SIZE); env->pf->pfte.rs_num = PF_RULESET_RDR; if (ioctl(env->pf->dev, DIOCXBEGIN, &env->pf->pft) == -1) @@ -254,13 +284,23 @@ sync_ruleset(struct hoststated *env, struct service *service, int enable) char anchor[PF_ANCHOR_NAME_SIZE]; bzero(anchor, sizeof(anchor)); - (void)strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)); - (void)strlcat(anchor, service->name, sizeof(anchor)); - transaction_init(env, anchor); + if (strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(anchor, service->name, sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (transaction_init(env, anchor) == -1) { + log_warn("sync_ruleset: transaction init failed"); + return; + } if (!enable) { - transaction_commit(env); - log_debug("sync_ruleset: rules removed"); + if (transaction_commit(env) == -1) + log_warn("sync_ruleset: " + "remove rules transaction failed"); + else + log_debug("sync_ruleset: rules removed"); return; } @@ -306,8 +346,9 @@ sync_ruleset(struct hoststated *env, struct service *service, int enable) } pio.addr.addr.type = PF_ADDR_TABLE; - (void)strlcpy(pio.addr.addr.v.tblname, service->name, - sizeof(pio.addr.addr.v.tblname)); + if (strlcpy(pio.addr.addr.v.tblname, service->name, + sizeof(pio.addr.addr.v.tblname)) >= sizeof(pio.addr.addr.v.tblname)) + fatal("sync_ruleset: table name too long"); if (ioctl(env->pf->dev, DIOCADDADDR, &pio) == -1) fatal("sync_ruleset: cannot add address to pool"); @@ -321,7 +362,12 @@ sync_ruleset(struct hoststated *env, struct service *service, int enable) fatal("cannot add rule"); log_debug("sync_ruleset: rule added"); } - transaction_commit(env); + if (transaction_commit(env) == -1) + log_warn("sync_ruleset: add rules transaction failed"); + return; + + toolong: + fatal("sync_ruleset: name too long"); } void @@ -332,13 +378,27 @@ flush_rulesets(struct hoststated *env) kill_tables(env); TAILQ_FOREACH(service, &env->services, entry) { - strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)); - strlcat(anchor, service->name, sizeof(anchor)); - transaction_init(env, anchor); - transaction_commit(env); + if (strlcpy(anchor, HOSTSTATED_ANCHOR "/", sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (strlcat(anchor, service->name, sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (transaction_init(env, anchor) == -1 || + transaction_commit(env) == -1) + log_warn("flush_rulesets: transaction for %s/ failed", + HOSTSTATED_ANCHOR); } - strlcpy(anchor, HOSTSTATED_ANCHOR, sizeof(anchor)); - transaction_init(env, anchor); - transaction_commit(env); + if (strlcpy(anchor, HOSTSTATED_ANCHOR, sizeof(anchor)) >= + PF_ANCHOR_NAME_SIZE) + goto toolong; + if (transaction_init(env, anchor) == -1 || + transaction_commit(env) == -1) + log_warn("flush_rulesets: transaction for %s failed", + HOSTSTATED_ANCHOR); log_debug("flush_rulesets: flushed rules"); + return; + + toolong: + fatal("flush_rulesets: name too long"); } diff --git a/usr.sbin/relayd/relayd.c b/usr.sbin/relayd/relayd.c index e0aa22192fe..976029c0e43 100644 --- a/usr.sbin/relayd/relayd.c +++ b/usr.sbin/relayd/relayd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: relayd.c,v 1.15 2007/02/07 13:30:17 reyk Exp $ */ +/* $OpenBSD: relayd.c,v 1.16 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -152,8 +152,10 @@ main(int argc, char *argv[]) if (getpwnam(HOSTSTATED_USER) == NULL) errx(1, "unknown user %s", HOSTSTATED_USER); - if (!debug) - daemon(1, 0); + if (!debug) { + if (daemon(1, 0) == -1) + err(1, "failed to daemonize"); + } log_info("startup"); diff --git a/usr.sbin/relayd/ssl.c b/usr.sbin/relayd/ssl.c index 87c887d5e21..ddac5c5974c 100644 --- a/usr.sbin/relayd/ssl.c +++ b/usr.sbin/relayd/ssl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl.c,v 1.5 2007/02/07 14:39:45 reyk Exp $ */ +/* $OpenBSD: ssl.c,v 1.6 2007/02/08 13:32:24 reyk Exp $ */ /* * Copyright (c) 2006 Pierre-Yves Ritschard <pyr@spootnik.org> @@ -94,8 +94,8 @@ ssl_read(int s, short event, void *arg) } return; } - buf_add(cte->buf, rbuf, ret); - + if (buf_add(cte->buf, rbuf, ret) == -1) + fatal("ssl_read: buf_add error"); if (cte->validate_read != NULL) { if (cte->validate_read(cte) != 0) goto retry; |