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