hi!

On Mon, Sep 08, 2008 at 09:21:44PM -0400, Nick Bender wrote:
quoted text
> Index: usr.sbin/dhcpd/options.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dhcpd/options.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 options.c
> --- usr.sbin/dhcpd/options.c	16 Apr 2008 00:36:48 -0000	1.21
> +++ usr.sbin/dhcpd/options.c	9 Sep 2008 01:12:27 -0000
> @@ -207,11 +207,11 @@ void
>  create_priority_list(unsigned char *priority_list, unsigned char *prl,
>      int prl_len)
>  {
> -	int stored_list[256];
> +	unsigned char stored_list[256];
>  	int i, priority_len = 0;
>  
> -	bzero(stored_list, 256);
> -	bzero(priority_list, 256);

indeed, this is ugly...

quoted text
> +	bzero(stored_list, sizeof(stored_list));
> +	bzero(priority_list, sizeof(priority_list));
>  

unfortunately this fix contains a failure, because you cannot do
sizeof(priority_list) here - it will return the size of the pointer.
you cannot actually know the size of the complete array here and it is
the uglyness of the dhcpd code itself that it is using a hardcoded
size "256" everywhere instead of a) passing passing the output buffer
size as well or b) using at least a human-readable define like
"#define DHCP_OPTIONS 256".

the attached diff would clear the buffer before calling the
create_priority_list() function.

reyk

Index: options.c
===================================================================
RCS file: /cvs/src/usr.sbin/dhcpd/options.c,v
retrieving revision 1.21
diff -u -p -r1.21 options.c
--- options.c	16 Apr 2008 00:36:48 -0000	1.21
+++ options.c	9 Sep 2008 21:15:19 -0000
@@ -207,11 +207,11 @@ void
 create_priority_list(unsigned char *priority_list, unsigned char *prl,
     int prl_len)
 {
-	int stored_list[256];
+	unsigned char stored_list[256];
 	int i, priority_len = 0;
 
-	bzero(stored_list, 256);
-	bzero(priority_list, 256);
+	/* clear stored_list, priority_list should be cleared before */
+	bzero(&stored_list, sizeof(stored_list));
 
 	/* Some options we don't want on the priority list. */
 	stored_list[DHO_PAD] = 1;
@@ -304,6 +304,7 @@ cons_options(struct packet *inpacket, st
 	 * list provided in the options. Lacking that use the list provided by
 	 * prl. If that is not available just use the default list.
 	 */
+	bzero(&priority_list, sizeof(priority_list));
 	if (inpacket && inpacket->options[DHO_DHCP_PARAMETER_REQUEST_LIST].data)
 		create_priority_list(priority_list,
 		    inpacket->options[DHO_DHCP_PARAMETER_REQUEST_LIST].data,