Talk:Buffer overflow/GA1

GA Review

Article (edit | visual edit | history) · Article talk (edit | history) · Watch

Reviewer: Maury Markowitz (talk · contribs) 21:34, 20 October 2016 (UTC)[reply]

Ok, here we go...

"Buffer overflows can be triggered by inputs that are designed to execute code" - this appears to be mixing two separate things. One is that buffer overflows can trigger code execution, and the other is that you may design a program to do that deliberately. Putting them both together in a single statement seems confusing.
"Bounds checking can prevent buffer overflows" - indeed, but it can also significantly effect performance. AFAIK this is the reason C++ did not add it, and I think that is important to mention even in the lede.
I really like the example section! My only concern here is the end, which seems WAY too inside baseball for this article?
"by overwriting a local variable that is near the buffer in memory on the stack to change the behavior of the program" - this does not parse... near...in...on...to change... Perhaps this can be rewritten?
"With a method called "trampolining"" - I would recommend re-writing this statement, it is very confusing to me in its current form. Particularity, "if the address of the user-supplied data is unknown, but the location is stored in a register" I can't seem to understand. Perhaps this can be (greatly) simplified?
"Stack-based buffer overflows are not to be confused with stack overflows." - why is this statement here?
"Also note that these vulnerabilities" - no! A fuzzer is worth mentioning, but that should definitely be in the Protective countermeasures, not just sort of floating about here.
"Barriers to exploitation" - this seems out of place too. It would seem this is a protective countermeasure too.
"NOP-sled" - is it NOP-sled or NOP slide? Suggest picking the later unless there's a good reason not to.

More later, gotta put the kid to bed. Maury Markowitz (talk) 23:56, 20 October 2016 (UTC)[reply]

Thanks for reviewing this! I nominated it because I couldn't find any significant flaws, so it's really great to get another pair of eyes on it. To your comments:

I've updated the lede to address your first two concerns. The topic of buffer overflow exploitability prevention is complex (possibly one of the most complex things we have), but I think of all the things to mention in the lede that is it. Of course there are other reasons C++ doesn't add intrinsic bounds checking, but x86 now has extensions for it... anyway:
I like that the example is concrete. I think if I didn't know what a buffer overflow was at all or why it was a problem, that example would do a really good job of helping me understand. I think we can probably assume someone interested in this topic has enough C to understand it? I'm not sure what else could be used to mediate the explanation - certainly it could be written in Pascal, but I think people are less likely to understand it then.
I've made some minor changes for clarity to the stack-based examples.
Reference the trampoline, yes, when I think about it, a lot of stuff is going on there. Usually exploitation now involves an improbable number of steps and many interacting components, but I've simplified it as best I can.
Yes, you're right about fuzzing! I moved it to a new section in the mitigations part.

The remainder needs more work than I can do right this moment; more to follow. FalconK (talk) 11:16, 22 October 2016 (UTC)[reply]