A basic implementation for increased security in wine proposal

[Guillaume SH]

Hi Paul,

You asked me to actually describe the security I am concerned about, so I am
going for it :

Imagine an ill-intentioned people, call it the attackers. By the mean of
simply creating the following C application (based on classical "Hello
word") :


#include needed header

int main (int argc, char * argv[])
{
    /* printf ( "Hello world!" ); */
    GetOverlappedResult(0, NULL, NULL, FALSE);

    return EXIT_SUCCESS;
}


Running this application on wine, I get to have my crash, with the
possibility of an exploit. So all I have to do know is to find a vector to
make you and some other people willing to run my application.

I won't describe in detail the way to perform the exploit as :
      1 - I don't know how to proceed and I don't want to
      2 - It would be showing poor sense of responsibilities

Guillaume

---

2009/2/1 Paul TBBle Hampson <Paul.Hampson at pobox.com>

> On Sun, Feb 01, 2009 at 09:11:29AM +0100, Guillaume SH wrote:
> > I tested the two modes with the help of wine test suite, restricted to
> > kernel/file.c, test_overlapped and I considered only :
> >     all must-be-successful tests
> >     GetOverlappedResult(0, NULL, &result, FALSE);
> >     GetOverlappedResult(0, &ov, NULL, FALSE);
> > (these last two cannot be used in the same run, in either mode)
>
> This is potentially a flaw in the tests. The NULL-passing-in tests
> should have an exception handler wrapped around them if they are
> expected to throw an exception, an example of such was posted to this
> mailing list during an earlier discussion of this method's failure
> cases.
>
> If your automated test suite itself is crashing, you're not actually
> getting useful results.
>
> > The problem I am considering (and I may be mistaken here as I am not an
> > expert) is called "Unchecked Error Condition"(2)
> > and it is a referenced weakness (CWE id 391), rated "Medium" in
> likelihood
> > of exploit, by serious people.
>
> > (2) http://cwe.mitre.org/data/definitions/391.html
>
> This isn't an Unchecked Error Condition in GetOverlappedResult. It's a
> known and understood invalid input (NULL for something that the API
> specifies cannot be NULL) with an empirically determined failure action
> (throw a NULL-reference exception).
>
> An Unchecked Error Condition would occur if someone specifically wrote
> code around a GetOverlappedResult to catch the NULL-reference, and then
> ignored it and preceded on as if it hadn't given a NULL-reference.
>
> It would (at least in according to my understanding of the reasoning
> behind the Win32 API) be much more common for people to accidentally
> produce the Unchecked Error Condition flaw if a NULL going into these
> inputs (which the API specifies is invalid) were to produce an error
> code.
>
> Exceptions catch programmer's attention, they have to work to
> ignore them. They are "exceptional" events.
>
> Error codes get ignored left, right and centre.
>
> I can't see how GetOverlappedResult could be a security weakness.
>
> Program does dumb thing -> program crashes. It's not executing
> user-provided code or jumping about the stack in any way. It just
> bubbles straight up to the structured exception handler if one exists,
> or away into the night if one doesn't. Either way the code flow is
> interrupted, not redirected.
>
> If an attacker is limited to zeroing out a chunk of your stack, such
> that an otherwise-valid pointer becomes NULL and gets passed into
> GetOverlappedResult, you don't want to keep processing your
> error-handling path. Who know what other data the attacker was able to
> get onto your stack or heap? Instead, throw an exception and let the
> exception handling code (which should make significantly fewer
> assumptions about what state is valid) clean up for you.
>
> And in that case, the security issue is whatever the attacker used to
> zero out chunks of your stack.
>
> Can you actually please describe the security issue you're trying to fix
> here?
>
> > PS : I have observed that you are very active about fixing other security
> > issues :
> >         + buffer overflow
> >         + NULL pointer dereference
> >         + variable signedness
> >         + ...
> > I seems to me that hopefully, you performs those fixes without
> consideration
> > for "Is windows doing the same ?". I will be sad the day I will see a
> commit
> > "Remove resource release because Windows is doing the same" or "Remove
> > buffer overflow fix because app A (2 million copies sold) rely on it".
>
> Have a look at the ddrawex implementation. It's basically ddraw except
> it doesn't release resources as soon as they're released by the caller
> due to the way Internet Explorer and its plugins interact with the
> library. (Or at least that's the understanding I have from skimming the
> submitted patch)
>
> It'd be a sad day when we start sacrificing Win32 API implementation
> accuracy just because we think the Win32 API is wrong.
>
> --
> -----------------------------------------------------------
> Paul "TBBle" Hampson, B.Sc, LPI, MCSE
> Very-later-year Asian Studies student, ANU
> The Boss, Bubblesworth Pty Ltd (ABN: 51 095 284 361)
> Paul.Hampson at Pobox.com
>
> Of course Pacman didn't influence us as kids. If it did,
> we'd be running around in darkened rooms, popping pills and
> listening to repetitive music.
>  -- Kristian Wilson, Nintendo, Inc, 1989
>
> License: http://creativecommons.org/licenses/by/2.5/au/
> -----------------------------------------------------------
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.winehq.org/pipermail/wine-devel/attachments/20090201/c36cacc6/attachment.htm