Re: [ros-diffs] [amunger] 21474: DHCP Client fixes and enhancements Set the correct hardware type flag. Fixes bug 651. Send option 12 on discovery, allows some DHCP servers to dynamically update DNS. Send option 61 for good measure. We use the MAC address as some other OSes do.

[Thomas Weidenmueller]

There are several issues with this patch:
>   +       /* What a strage dance */
>  +       struct client_config *config = ifi->client->config; 
1) dhcp crashes for me on the line above.
>   +       char ComputerName [MAX_COMPUTERNAME_LENGTH +
> 1]; 
2) use Unicode, please. Obsolete as described in 4)
>   +       DWORD ComputerNameSize = sizeof ComputerName /
> sizeof ComputerName[0];
>  +
>  +       GetComputerName(ComputerName, & ComputerNameSize); 
3) missing error check, leading to buffer overflow if accessed the
string in ComputerName
>   +       /* This never gets freed since it's only
> called once */
>  +       LPSTR lpCompName =
>  +       HeapAlloc(GetProcessHeap(), 0, strlen(ComputerName) + 1); 
4) makes the ComputerName buffer on the stack obsolete. Only use the
dynamic buffer. strlen will likely cause a buffer overflow if the
computer name was longer than the length of the (obsolete) static buffer
on the stack, since the static buffer will never be initialized in this
case.
>   +       if (lpCompName !=NULL) {
>  +           GetComputerName(lpCompName, & ComputerNameSize); 
5) once again missing error check which may cause buffer overflows in
the following code
>   +           /* Send our hostname, some dhcpds use this
> to update DNS */
>  +           config->send_options[DHO_HOST_NAME].data = strlwr(lpCompName); 
6) strlwr is POSIX ;)
>   +           config->send_options[DHO_HOST_NAME].len
> = strlen(ComputerName); 
7) operating on the wrong buffer, may cause buffer overflow due to 5)
>   +       warn("util.c read_client_conf poorly
> implemented!"); 
Indeed :(
Apart from the bugs mentioned in this code, GetComputerNameExA has a few
bugs: 1) incorrectly returning ERROR_OUTOFMEMORY instead of FALSE and 2)
not checking the return value of RtlUnicodeStringToAnsiString.
- Thomas