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