On 4/6/06, Thomas Weidenmueller <w3seek(a)reactos.com> wrote:
> 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.
Strange, it works for me here.
>
> > + char ComputerName [MAX_COMPUTERNAME_LENGTH + 1];
>
> 2) use Unicode, please. Obsolete as described in 4)
I really tire of hearing this. None of the code in dhcp.exe is
unicode right now. I've tried to hack on several pieces of ros only
to be told "USE UNICODE" by another dev. Trying to convert entire
modules has always gone terribly for me.
>
> > + 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.
I'll try to fix this.
>
> > + 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 ;)
Not helpful, suggest something better.
>
> > + 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
>
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
WD
--
<Bizzeh> it even has a panda that pops out of your case and mauls
people who ask stupid questions
Thomas Weidenmueller wrote:
> I don't mean to offend anybody with my criticism, I just want to point
> out what could and should be fixed or done differently.
Your criticism is hugely appreciated. That's the whole point of ros-diffs :)
WaxDragon wrote:
> I'll try to fix this.
I'll give you a hand with it tonight if you need one.
Ged.
************************************************************************
The information contained in this message or any of its
attachments is confidential and is intended for the exclusive
use of the addressee. The information may also be legally
privileged. The views expressed may not be company policy,
but the personal views of the originator. If you are not the
addressee, any disclosure, reproduction, distribution or other
dissemination or use of this communication is strictly prohibited.
If you have received this message in error, please contact
postmaster(a)exideuk.co.uk
<mailto:postmaster@exideuk.co.uk> and then delete this message.
Exide Technologies is an industrial and transportation battery
producer and recycler with operations in 89 countries.
Further information can be found at www.exide.com
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
> From: "Murphy, Ged (Bolton)" <MurphyG(a)cmpbatteries.co.uk>
>
> I'm going to have a look at this tonight, but I've had a look
> before and IIRC, I had trouble hunting the bug down.
> It would be good if someone with a bit more experience in
> this area could have a look.
>
> I think Ge was working on this before Christmas,
I was
> so I'm
> hopefully gonna catch up with him tonight via IM, as I'm not
> sure if he's subscribed to ros-dev anymore.
I am, although I don't read every message anymore.
The status of the applet when I worked on it was that the UI was complete
and I was starting to work on transfering the data from the dialog to the
TCP/IP driver (via the DHCP client). That part simply isn't implemented yet.
I don't have any uncommitted code floating around.
Ge
> + DWORD HELMemoryAvilable;
> + This->HELMemoryAvilable = HEL_GRAPHIC_MEMORY_MAX;
> + *free = This->HELMemoryAvilable;
There is a small typo throughout this code
"Available"
:)
************************************************************************
The information contained in this message or any of its
attachments is confidential and is intended for the exclusive
use of the addressee. The information may also be legally
privileged. The views expressed may not be company policy,
but the personal views of the originator. If you are not the
addressee, any disclosure, reproduction, distribution or other
dissemination or use of this communication is strictly prohibited.
If you have received this message in error, please contact
postmaster(a)exideuk.co.uk
<mailto:postmaster@exideuk.co.uk> and then delete this message.
Exide Technologies is an industrial and transportation battery
producer and recycler with operations in 89 countries.
Further information can be found at www.exide.com
aleksey(a)studiocerebral.com wrote:
> Author: tretiakov
> Date: Fri Mar 31 21:47:52 2006
> New Revision: 21429
>
> URL: http://svn.reactos.ru/svn/reactos?rev=21429&view=rev
> Log:
> [AUDIT] msgina is clean.
>
>
What is the reason for these being clean signed off as clean?
Did you speak to the authors? Did you follow the conditions in the wiki?
aleksey(a)studiocerebral.com wrote:
> -reactos/dll/win32/comctl32 # Synced to Wine-0_9_5
> +reactos/dll/win32/comctl32 # Synced to Wine-20060328
The "synchronization" lacks a bunch of changes. Only a few files were
actually synchronized.
- Thomas
> Hey if you wanted to part with the RS/6000 arty might be interested as
> he has been doing a good bit on the powerpc port. His Mac just died
> but he bought one of those nice IBM AIX Laptops that can run NT4 and
> is working on that.
Not quite ready to part with that thing yet. I'm having a house built,
so I'll again have my own office, so I'll be able to hook it and the
Alpha up via KVM on its own desk again. Part of the reason its packed
up now is I'm living in a little tin can of a place in the meantime.
> Yes it does. Would you like me to bring it down to you? Your in the
> South East right?
Yea I'm over in GA.
If the machine has built in USB then that would be very handy to allow
me to do some debugging on prebuilt hardware instead of sticking a
generic USB card in there. I may actually be coming over to SC this
weekend
Laters,
mike
Lol, you still have that Alpha machine. My PWS 433a is still sitting in
my backroom where I wanted a port of ReactOS for so very long, but could
never devote enough time to figuring the technical details out.
Another interesting note is I still have the old RS/6000 that Eric and I
were trying to get ROS to run on PPC too, however its packed up in the
closet below my even older AlphaStation 4/233!
Does your PWS433 have USB? My latest quest has been to hack USB support
for AlphaNT, but thus far its not worked out very well, lol.
Good Luck,
Mike
-----Original Message-----
From: ros-dev-bounces(a)reactos.org [mailto:ros-dev-bounces@reactos.org]
On Behalf Of Steven Edwards
Sent: Tuesday, March 14, 2006 2:42 PM
To: ros-dev(a)reactos.org
Subject: [ros-dev] Alpha workstation
Hi,
Is anyone in the US interested in using the Alpha workstation for a
possible port? Its kind of gathering dust here and if no one wants it
I would rather we just sell it and stick the money in to the project
pot.