Re: PATCH wine/controls/button.c paint_button() : "style" range checking

[Alexandre Julliard]

Vincent Pelletier <subdino2004(a)yahoo.fr> writes:
> Checks for out-of-range value in "style" (paint_button function):
> must be style<MAX_BTN_TYPE or memory violation (reading unpredictable
> btyes) occurs, and as it's then used as a function address that could
> lead to random code execution.

If you really want an assertion you should use assert(), at least then
it can be compiled out. And if it's a condition that can legitimately
happen, then it has to be handled properly, just adding an ERR doesn't
improve anything.  In this case I guess it's conceivable that the app
would change the style to an invalid one, so it needs to be handled.

-- 
Alexandre Julliard
julliard(a)winehq.org

PATCH wine/controls/button.c paint_button() : "style" range checking

[Vincent Pelletier]

Alexandre Julliard wrote:
> If you really want an assertion you should use assert(), at least then
> it can be compiled out. And if it's a condition that can legitimately
> happen, then it has to be handled properly, just adding an ERR doesn't
> improve anything.  In this case I guess it's conceivable that the app
> would change the style to an invalid one, so it needs to be handled.

I wanted to make it somewhat friendlier than a simple message & exit. Or 
I may missunderstand what assert does...
At least, it could be used as a security against that "random code 
execution", and at most it could help devs tracking bugs.
Btw, I suggest adding the source path (relative to wine's root of 
course) & function name in every error message, at least when run in 
trace mode. (I can take care of it if necessary.)

PATCH wine/controls/button.c paint_button() : "style" range checking

[Mike Hearn]

On Thu, 19 Feb 2004 18:55:19 +0100, Vincent Pelletier wrote:
> Btw, I suggest adding the source path (relative to wine's root of 
> course) & function name in every error message, at least when run in 
> trace mode. (I can take care of it if necessary.)

I don't think there's any need for that - a trace message typically
includes the function name. You can use tools/findfunc or the tags feature
of your editor to find the relevant part of the code.

I know the Wine sources seem rather large and confusing at first, but you
do get used to it fast - promise! The last thing we want is for logs to
become even more unwieldy though, they are already large enough.

thanks -mike

PATCH wine/controls/button.c paint_button() : "style" range checking

[Vincent Pelletier]

Mike Hearn wrote:
> The last thing we want is for logs to
> become even more unwieldy though, they are already large enough.

Hum, right. Nevermind :$ :).