diff options
| -rw-r--r-- | RELNOTES | 3 | ||||
| -rw-r--r-- | common/bpf.c | 5 | ||||
| -rw-r--r-- | common/dns.c | 8 | ||||
| -rw-r--r-- | common/parse.c | 5 | ||||
| -rw-r--r-- | common/print.c | 9 | ||||
| -rw-r--r-- | common/tree.c | 11 | ||||
| -rw-r--r-- | common/upf.c | 6 | ||||
| -rw-r--r-- | omapip/errwarn.c | 14 | ||||
| -rw-r--r-- | server/bootp.c | 5 | ||||
| -rw-r--r-- | server/db.c | 37 | ||||
| -rw-r--r-- | server/ddns.c | 15 | ||||
| -rw-r--r-- | server/dhcp.c | 31 | ||||
| -rw-r--r-- | server/failover.c | 68 |
13 files changed, 160 insertions, 57 deletions
@@ -60,7 +60,8 @@ and for prodding me into improving it. snprintf implementation, only in those cases where the architecture is not known to provide one (see includes/cf/[arch].h). If you experience linking problems with snprintf/vsnprintf or 'isc_print_' functions, this - is where to look. + is where to look. This vulnerability did not exist in any previously + published version of ISC DHCP. - Compilation on hpux 11.11 was repaired. diff --git a/common/bpf.c b/common/bpf.c index 24e11759..9c321533 100644 --- a/common/bpf.c +++ b/common/bpf.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: bpf.c,v 1.48.2.5 2004/06/14 21:08:42 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: bpf.c,v 1.48.2.6 2004/06/17 20:54:38 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -90,7 +90,8 @@ int if_register_bpf (info) /* Open a BPF device */ for (b = 0; 1; b++) { - snprintf(filename, sizeof(filename), BPF_FORMAT, b); + /* %Audit% 31 bytes max. %2004.06.17,Safe% */ + sprintf(filename, BPF_FORMAT, b); sock = open (filename, O_RDWR, 0); if (sock < 0) { if (errno == EBUSY) { diff --git a/common/dns.c b/common/dns.c index ea4f3a5d..e80df9ab 100644 --- a/common/dns.c +++ b/common/dns.c @@ -33,7 +33,7 @@ #ifndef lint static char copyright[] = -"$Id: dns.c,v 1.35.2.15 2004/06/14 21:08:42 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: dns.c,v 1.35.2.16 2004/06/17 20:54:38 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -531,7 +531,8 @@ isc_result_t ddns_update_a (struct data_string *ddns_fwd_name, if (ddns_addr.len != 4) return ISC_R_INVALIDARG; - snprintf (ddns_address, 16, "%d.%d.%d.%d", + /* %Audit% Cannot exceed 16 bytes. %2004.06.17,Safe% */ + sprintf (ddns_address, "%u.%u.%u.%u", ddns_addr.iabuf[0], ddns_addr.iabuf[1], ddns_addr.iabuf[2], ddns_addr.iabuf[3]); @@ -779,7 +780,8 @@ isc_result_t ddns_remove_a (struct data_string *ddns_fwd_name, if (ddns_addr.len != 4) return ISC_R_INVALIDARG; - snprintf (ddns_address, 16, "%d.%d.%d.%d", + /* %Audit% Cannot exceed 16 bytes. %2004.06.17,Safe% */ + sprintf (ddns_address, "%u.%u.%u.%u", ddns_addr.iabuf[0], ddns_addr.iabuf[1], ddns_addr.iabuf[2], ddns_addr.iabuf[3]); diff --git a/common/parse.c b/common/parse.c index 0a119472..54394f22 100644 --- a/common/parse.c +++ b/common/parse.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: parse.c,v 1.104.2.16 2004/06/14 21:08:43 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: parse.c,v 1.104.2.17 2004/06/17 20:54:38 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -4796,6 +4796,9 @@ int parse_warn (struct parse *cfile, const char *fmt, ...) unsigned i, lix; do_percentm (mbuf, fmt); + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ snprintf (fbuf, sizeof fbuf, "%s line %d: %s", cfile -> tlname, cfile -> lexline, mbuf); diff --git a/common/print.c b/common/print.c index 4daaa328..dba11703 100644 --- a/common/print.c +++ b/common/print.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: print.c,v 1.53.2.10 2004/06/10 17:59:20 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: print.c,v 1.53.2.11 2004/06/17 20:54:39 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -414,8 +414,13 @@ char *print_dotted_quads (len, data) i = 0; + /* %Audit% Loop bounds checks to 21 bytes. %2004.06.17,Safe% + * The sprintf can't exceed 18 bytes, and since the loop enforces + * 21 bytes of space per iteration at no time can we exit the + * loop without at least 3 bytes spare. + */ do { - sprintf (s, "%d.%d.%d.%d, ", + sprintf (s, "%u.%u.%u.%u, ", data [i], data [i + 1], data [i + 2], data [i + 3]); s += strlen (s); i += 4; diff --git a/common/tree.c b/common/tree.c index 6dba8f32..ab357fb9 100644 --- a/common/tree.c +++ b/common/tree.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: tree.c,v 1.101.2.8 2004/06/10 17:59:22 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: tree.c,v 1.101.2.9 2004/06/17 20:54:39 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -735,10 +735,13 @@ int evaluate_dns_expression (result, packet, lease, client_state, in_options, goto dpngood; (*result) -> r_data = (*result) -> r_data_ephem; + /*%Audit% 16 bytes max. %2004.06.17,Safe%*/ sprintf ((char *)(*result) -> r_data_ephem, - "%d.%d.%d.%d", - data.data [0], data.data [1], - data.data [2], data.data [3]); + "%u.%u.%u.%u", + data.data [0] & 0xff, + data.data [1] & 0xff, + data.data [2] & 0xff, + data.data [3] & 0xff); (*result) -> r_size = strlen ((const char *) (*result) -> r_data); diff --git a/common/upf.c b/common/upf.c index 8d01df28..72e1b29d 100644 --- a/common/upf.c +++ b/common/upf.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: upf.c,v 1.21.2.3 2004/06/14 21:08:44 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: upf.c,v 1.21.2.4 2004/06/17 20:54:39 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -79,7 +79,9 @@ int if_register_upf (info) /* Open a UPF device */ for (b = 0; 1; b++) { - snprintf(filename, sizeof(filename), "/dev/pf/pfilt%d", b); + /* %Audit% Cannot exceed 36 bytes. %2004.06.17,Safe% */ + sprintf(filename, "/dev/pf/pfilt%d", b); + sock = open (filename, O_RDWR, 0); if (sock < 0) { if (errno == EBUSY) { diff --git a/omapip/errwarn.c b/omapip/errwarn.c index 899b1596..8513a44f 100644 --- a/omapip/errwarn.c +++ b/omapip/errwarn.c @@ -33,7 +33,7 @@ #ifndef lint static char copyright[] = -"$Id: errwarn.c,v 1.9.2.1 2004/06/10 17:59:47 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: errwarn.c,v 1.9.2.2 2004/06/17 20:54:39 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include <omapip/omapip_p.h> @@ -59,6 +59,9 @@ void log_fatal (const char * fmt, ... ) do_percentm (fbuf, fmt); + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ va_start (list, fmt); vsnprintf (mbuf, sizeof mbuf, fbuf, list); va_end (list); @@ -105,6 +108,9 @@ int log_error (const char * fmt, ...) do_percentm (fbuf, fmt); + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ va_start (list, fmt); vsnprintf (mbuf, sizeof mbuf, fbuf, list); va_end (list); @@ -129,6 +135,9 @@ int log_info (const char *fmt, ...) do_percentm (fbuf, fmt); + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ va_start (list, fmt); vsnprintf (mbuf, sizeof mbuf, fbuf, list); va_end (list); @@ -153,6 +162,9 @@ int log_debug (const char *fmt, ...) do_percentm (fbuf, fmt); + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ va_start (list, fmt); vsnprintf (mbuf, sizeof mbuf, fbuf, list); va_end (list); diff --git a/server/bootp.c b/server/bootp.c index 2ea546e4..33445de1 100644 --- a/server/bootp.c +++ b/server/bootp.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: bootp.c,v 1.69.2.6 2004/06/15 16:15:58 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: bootp.c,v 1.69.2.7 2004/06/17 20:54:39 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -68,6 +68,9 @@ void bootp (packet) if (packet -> raw -> op != BOOTREQUEST) return; + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ snprintf (msgbuf, sizeof msgbuf, "BOOTREQUEST from %s via %s", print_hw_addr (packet -> raw -> htype, packet -> raw -> hlen, diff --git a/server/db.c b/server/db.c index 22377565..614d6a01 100644 --- a/server/db.c +++ b/server/db.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: db.c,v 1.63.2.10 2004/06/15 16:15:58 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: db.c,v 1.63.2.11 2004/06/17 20:54:40 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -80,6 +80,7 @@ int write_lease (lease) if (lease -> starts) { if (lease -> starts != MAX_TIME) { t = gmtime (&lease -> starts); + /* %Audit% Cannot exceed 59 bytes. %2004.06.17,Safe% */ sprintf (tbuf, "%d %d/%02d/%02d %02d:%02d:%02d;", t -> tm_wday, t -> tm_year + 1900, t -> tm_mon + 1, t -> tm_mday, @@ -96,6 +97,7 @@ int write_lease (lease) if (lease -> ends) { if (lease -> ends != MAX_TIME) { t = gmtime (&lease -> ends); + /* %Audit% Cannot exceed 59 bytes. %2004.06.17,Safe% */ sprintf (tbuf, "%d %d/%02d/%02d %02d:%02d:%02d;", t -> tm_wday, t -> tm_year + 1900, t -> tm_mon + 1, t -> tm_mday, @@ -401,11 +403,11 @@ int write_host (host) } for (i = 0; i < ip_addrs.len - 3; i += 4) { errno = 0; - fprintf (db_file, "%d.%d.%d.%d%s", - ip_addrs.data [i], - ip_addrs.data [i + 1], - ip_addrs.data [i + 2], - ip_addrs.data [i + 3], + fprintf (db_file, "%u.%u.%u.%u%s", + ip_addrs.data [i] & 0xff, + ip_addrs.data [i + 1] & 0xff, + ip_addrs.data [i + 2] & 0xff, + ip_addrs.data [i + 3] & 0xff, i + 7 < ip_addrs.len ? "," : ""); if (errno) { ++errors; @@ -773,7 +775,17 @@ int new_lease_file () /* Make a temporary lease file... */ GET_TIME (&t); - snprintf (newfname, sizeof newfname, "%s.%d", path_dhcpd_db, (int)t); + + /* %Audit% Truncated filename causes panic. %2004.06.17,Safe% + * This should never happen since the path is a configuration + * variable from build-time or command-line. But if it should, + * either by malice or ignorance, we panic, since the potential + * for havoc is high. + */ + if (snprintf (newfname, sizeof newfname, "%s.%d", + path_dhcpd_db, (int)t) >= sizeof newfname) + log_fatal("new_lease_file: lease file path too long"); + db_fd = open (newfname, O_WRONLY | O_TRUNC | O_CREAT, 0664); if (db_fd < 0) { log_error ("Can't create new lease file: %m"); @@ -823,8 +835,17 @@ int new_lease_file () #if defined (TRACING) if (!trace_playback ()) { #endif + /* %Audit% Truncated filename causes panic. %2004.06.17,Safe% + * This should never happen since the path is a configuration + * variable from build-time or command-line. But if it should, + * either by malice or ignorance, we panic, since the potential + * for havoc is too high. + */ + if (snprintf (backfname, sizeof backfname, "%s~", path_dhcpd_db) + >= sizeof backfname) + log_fatal("new_lease_file: backup lease file path too long"); + /* Get the old database out of the way... */ - snprintf (backfname, sizeof backfname, "%s~", path_dhcpd_db); if (unlink (backfname) < 0 && errno != ENOENT) { log_error ("Can't remove old lease database backup %s: %m", backfname); diff --git a/server/ddns.c b/server/ddns.c index 24a6dd72..d5b46bb2 100644 --- a/server/ddns.c +++ b/server/ddns.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: ddns.c,v 1.15.2.13 2004/06/14 21:08:50 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: ddns.c,v 1.15.2.14 2004/06/17 20:54:40 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -496,12 +496,13 @@ int ddns_updates (struct packet *packet, if (ddns_rev_name.buffer) { ddns_rev_name.data = ddns_rev_name.buffer -> data; - snprintf ((char *)ddns_rev_name.buffer -> data, 17, - "%d.%d.%d.%d.", - lease -> ip_addr . iabuf[3], - lease -> ip_addr . iabuf[2], - lease -> ip_addr . iabuf[1], - lease -> ip_addr . iabuf[0]); + /* %Audit% Cannot exceed 17 bytes. %2004.06.17,Safe% */ + sprintf ((char *)ddns_rev_name.buffer -> data, + "%u.%u.%u.%u.", + lease -> ip_addr . iabuf[3] & 0xff, + lease -> ip_addr . iabuf[2] & 0xff, + lease -> ip_addr . iabuf[1] & 0xff, + lease -> ip_addr . iabuf[0] & 0xff); ddns_rev_name.len = strlen ((const char *)ddns_rev_name.data); diff --git a/server/dhcp.c b/server/dhcp.c index 1c24c901..6f1d2554 100644 --- a/server/dhcp.c +++ b/server/dhcp.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: dhcp.c,v 1.192.2.34 2004/06/15 16:15:58 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: dhcp.c,v 1.192.2.35 2004/06/17 20:54:40 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -80,8 +80,8 @@ void dhcp (packet) packet -> packet_type < dhcp_type_name_max - 1) { s = dhcp_type_names [packet -> packet_type - 1]; } else { - snprintf (typebuf, sizeof typebuf, - "type %d", packet -> packet_type); + /* %Audit% Cannot exceed 28 bytes. %2004.06.17,Safe% */ + sprintf (typebuf, "type %d", packet -> packet_type); s = typebuf; } @@ -263,7 +263,9 @@ void dhcpdiscover (packet, ms_nulltp) } else s = (char *)0; - /* Say what we're doing... */ + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ snprintf (msgbuf, sizeof msgbuf, "DHCPDISCOVER from %s %s%s%svia %s", (packet -> raw -> htype ? print_hw_addr (packet -> raw -> htype, @@ -455,12 +457,17 @@ void dhcprequest (packet, ms_nulltp, ip_lease) sip.len = 4; memcpy (sip.iabuf, data.data, 4); data_string_forget (&data, MDL); + /* piaddr() should not return more than a 15 byte string. + * safe. + */ sprintf (smbuf, " (%s)", piaddr (sip)); have_server_identifier = 1; } else smbuf [0] = 0; - /* Say what we're doing... */ + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ snprintf (msgbuf, sizeof msgbuf, "DHCPREQUEST for %s%s from %s %s%s%svia %s", piaddr (cip), smbuf, @@ -744,10 +751,16 @@ void dhcprelease (packet, ms_nulltp) } else s = (char *)0; + /* %Audit% Cannot exceed 16 bytes. %2004.06.17,Safe% + * We copy this out to stack because we actually want to log two + * inet_ntoa()'s in this message. + */ strncpy(cstr, inet_ntoa (packet -> raw -> ciaddr), 15); cstr[15] = '\0'; - /* Say what we're doing... */ + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ snprintf (msgbuf, sizeof msgbuf, "DHCPRELEASE of %s from %s %s%s%svia %s (%sfound)", cstr, @@ -835,6 +848,9 @@ void dhcpdecline (packet, ms_nulltp) } else s = (char *)0; + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ snprintf (msgbuf, sizeof msgbuf, "DHCPDECLINE of %s from %s %s%s%svia %s", piaddr (cip), @@ -947,6 +963,9 @@ void dhcpinform (packet, ms_nulltp) memcpy (cip.iabuf, &packet -> raw -> ciaddr, 4); } + /* %Audit% This is log output. %2004.06.17,Safe% + * If we truncate we hope the user can get a hint from the log. + */ snprintf (msgbuf, sizeof msgbuf, "DHCPINFORM from %s via %s", piaddr (cip), packet -> interface -> name); diff --git a/server/failover.c b/server/failover.c index 2d53f1e0..3ee72f04 100644 --- a/server/failover.c +++ b/server/failover.c @@ -34,7 +34,7 @@ #ifndef lint static char copyright[] = -"$Id: failover.c,v 1.53.2.30 2004/06/15 16:15:59 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; +"$Id: failover.c,v 1.53.2.31 2004/06/17 20:54:40 dhankins Exp $ Copyright (c) 2004 Internet Systems Consortium. All rights reserved.\n"; #endif /* not lint */ #include "dhcpd.h" @@ -466,7 +466,7 @@ isc_result_t dhcp_failover_link_signal (omapi_object_t *h, badconnect: /* XXX Send a refusal message first? XXX Look in protocol spec for guidance. */ - log_error ("Failover CONNECT from %d.%d.%d.%d: %s", + log_error ("Failover CONNECT from %u.%u.%u.%u: %s", ((u_int8_t *) (&link -> imsg -> server_addr)) [0], ((u_int8_t *) @@ -1246,7 +1246,7 @@ isc_result_t dhcp_failover_state_signal (omapi_object_t *o, link); if (link -> imsg -> reject_reason) { - log_error ("Failover CONNECT to %d.%d.%d.%d%s%s", + log_error ("Failover CONNECT to %u.%u.%u.%u%s%s", ((u_int8_t *) (&link -> imsg -> server_addr)) [0], ((u_int8_t *) @@ -1270,7 +1270,7 @@ isc_result_t dhcp_failover_state_signal (omapi_object_t *o, errmsg = "unknown server"; reason = FTR_INVALID_PARTNER; badconnectack: - log_error ("Failover CONNECTACK from %d.%d.%d.%d: %s", + log_error ("Failover CONNECTACK from %u.%u.%u.%u: %s", ((u_int8_t *) (&link -> imsg -> server_addr)) [0], ((u_int8_t *) @@ -1343,7 +1343,7 @@ isc_result_t dhcp_failover_state_signal (omapi_object_t *o, (tvunref_t)dhcp_failover_state_dereference); } else if (link -> imsg -> type == FTM_DISCONNECT) { if (link -> imsg -> reject_reason) { - log_error ("Failover DISCONNECT from %d.%d.%d.%d%s%s", + log_error ("Failover DISCONNECT from %u.%u.%u.%u%s%s", ((u_int8_t *) (&link -> imsg -> server_addr)) [0], ((u_int8_t *) @@ -3398,8 +3398,17 @@ failover_option_t *dhcp_failover_option_printf (unsigned code, va_list va; char tbuf [256]; + /* %Audit% Truncation causes panic. %2004.06.17,Revisit% + * It is unclear what the effects of truncation here are, or + * how that condition should be handled. It seems that this + * function is used for formatting messages in the failover + * command channel. For now the safest thing is for + * overflow-truncation to cause a fatal log. + */ va_start (va, fmt); - vsnprintf (tbuf, sizeof tbuf, fmt, va); + if (vsnprintf (tbuf, sizeof tbuf, fmt, va) >= sizeof tbuf) + log_fatal ("%s: vsnprintf would truncate", + "dhcp_failover_make_option"); va_end (va); return dhcp_failover_make_option (code, obuf, obufix, obufmax, @@ -3500,7 +3509,16 @@ failover_option_t *dhcp_failover_make_option (unsigned code, putUShort (&option.data [2], size - 4); #if defined (DEBUG_FAILOVER_MESSAGES) - snprintf (tbuf, sizeof tbuf, " (%s<%d>", info -> name, option.count); + /* %Audit% Truncation causes panic. %2004.06.17,Revisit% + * It is unclear what the effects of truncation here are, or + * how that condition should be handled. It seems that this + * message may be sent over the failover command channel. + * For now the safest thing is for overflow-truncation to cause + * a fatal log. + */ + if (snprintf (tbuf, sizeof tbuf, " (%s<%d>", info -> name, + option.count) >= sizeof tbuf) + log_fatal ("dhcp_failover_make_option: tbuf overflow"); failover_print (obuf, obufix, obufmax, tbuf); #endif @@ -3510,6 +3528,7 @@ failover_option_t *dhcp_failover_make_option (unsigned code, for (i = 0; i < count; i++) { val = va_arg (va, unsigned); #if defined (DEBUG_FAILOVER_MESSAGES) + /* %Audit% Cannot exceed 24 bytes. %2004.06.17,Safe% */ sprintf (tbuf, " %d", val); failover_print (obuf, obufix, obufmax, tbuf); #endif @@ -3529,8 +3548,9 @@ failover_option_t *dhcp_failover_make_option (unsigned code, } #if defined (DEBUG_FAILOVER_MESSAGES) - sprintf (tbuf, " %u.%u.%u.%u", iaddr [0], iaddr [1], - iaddr [2], iaddr [3]); + /*%Audit% Cannot exceed 17 bytes. %2004.06.17,Safe%*/ + sprintf (tbuf, " %u.%u.%u.%u", + iaddr [0], iaddr [1], iaddr [2], iaddr [3]); failover_print (obuf, obufix, obufmax, tbuf); #endif memcpy (&option.data [4 + i * ilen], iaddr, ilen); @@ -3541,6 +3561,7 @@ failover_option_t *dhcp_failover_make_option (unsigned code, for (i = 0; i < count; i++) { val = va_arg (va, unsigned); #if defined (DEBUG_FAILOVER_MESSAGES) + /*%Audit% Cannot exceed 24 bytes. %2004.06.17,Safe%*/ sprintf (tbuf, " %d", val); failover_print (obuf, obufix, obufmax, tbuf); #endif @@ -3553,6 +3574,7 @@ failover_option_t *dhcp_failover_make_option (unsigned code, bval = va_arg (va, u_int8_t *); #if defined (DEBUG_FAILOVER_MESSAGES) for (i = 0; i < count; i++) { + /* 23 bytes plus nul, safe. */ sprintf (tbuf, " %d", bval [i]); failover_print (obuf, obufix, obufmax, tbuf); } @@ -3561,17 +3583,21 @@ failover_option_t *dhcp_failover_make_option (unsigned code, break; /* On output, TEXT_OR_BYTES is _always_ text, and always NUL - terminated. Note that the caller should be careful not to - provide a format and data that amount to more than 256 bytes - of data, since it will be truncated on platforms that - support snprintf, and will mung the stack on those platforms - that do not support snprintf. Also, callers should not pass - data acquired from the network without specifically checking - it to make sure it won't bash the stack. */ + terminated. Note that the caller should be careful not + to provide a format and data that amount to more than 256 + bytes of data, since it will cause a fatal error. */ case FT_TEXT_OR_BYTES: case FT_TEXT: #if defined (DEBUG_FAILOVER_MESSAGES) - snprintf (tbuf, sizeof tbuf, "\"%s\"", txt); + /* %Audit% Truncation causes panic. %2004.06.17,Revisit% + * It is unclear what the effects of truncation here are, or + * how that condition should be handled. It seems that this + * function is used for formatting messages in the failover + * command channel. For now the safest thing is for + * overflow-truncation to cause a fatal log. + */ + if (snprintf (tbuf, sizeof tbuf, "\"%s\"", txt) >= sizeof tbuf) + log_fatal ("dhcp_failover_make_option: tbuf overflow"); failover_print (obuf, obufix, obufmax, tbuf); #endif memcpy (&option.data [4], txt, count); @@ -3586,6 +3612,7 @@ failover_option_t *dhcp_failover_make_option (unsigned code, memcpy (&option.data [4 + count], bval, size - count - 4); #if defined (DEBUG_FAILOVER_MESSAGES) for (i = 4; i < size; i++) { + /*%Audit% Cannot exceed 24 bytes. %2004.06.17,Safe%*/ sprintf (tbuf, " %d", option.data [i]); failover_print (obuf, obufix, obufmax, tbuf); } @@ -3596,6 +3623,7 @@ failover_option_t *dhcp_failover_make_option (unsigned code, for (i = 0; i < count; i++) { val = va_arg (va, u_int32_t); #if defined (DEBUG_FAILOVER_MESSAGES) + /*%Audit% Cannot exceed 24 bytes. %2004.06.17,Safe%*/ sprintf (tbuf, " %d", val); failover_print (obuf, obufix, obufmax, tbuf); #endif @@ -4480,11 +4508,13 @@ isc_result_t dhcp_failover_process_bind_update (dhcp_failover_state_t *state, if (new_binding_state != msg -> binding_status) { char outbuf [100]; - snprintf (outbuf, sizeof outbuf, + if (snprintf (outbuf, sizeof outbuf, "%s: invalid state transition: %s to %s", piaddr (lease -> ip_addr), binding_state_print (lease -> binding_state), - binding_state_print (msg -> binding_status)); + binding_state_print (msg -> binding_status)) + >= sizeof outbuf) + log_fatal ("%s: impossible outbuf overflow"); dhcp_failover_send_bind_ack (state, msg, FTR_FATAL_CONFLICT, |