6 Apr
2006
6 Apr
'06
10:38 a.m.
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