suprising memcpy wasn't flagged
http://blogs.msdn.com/sdl/archive/2007/04/26/lessons-learned-from-the-animated-cursor-security-bug.aspx
<quote>
In the Windows Vista process, we banned certain APIs, like strcpy and strncpy, and changed well over 140,000 calls to use safer calls. memcpy wasn’t on that list.
The bug is a stack-based buffer overrun in code that looks like this:
HICON LoadAniIcon() {
ANIHEADER myANIheader;
memcpy(&myANIheader, untrustedData->headerdata, untrustedData->headerlength);
</quote>
I've seen so many memcpy problems it's hard to imagine that it wasn't on their could be a problem list.
son of parnas
April 29th, 2007 5:12pm
I would imagine that maybe it was but that they removed it for performance reasons.
jollyJumper
April 29th, 2007 5:16pm
I created one of my worst bugs with memcpy, so maybe I am overly sensitive. But it's very dangerous to keep around. It's probably not a good idea to treat any memory as just a byte array. It should all be encaped by a type.
son of parnas
April 29th, 2007 5:25pm
heh. I would say that it was perfectly safe to treat memory as just a byte array, its just unsafe to make _any_ assumptions regarding the contents of that byte array.
jollyJumper
April 29th, 2007 5:39pm
bottom line, putting untrusted data into _anything_ is kind of stupid.
so I think I agree with you after all, all untrusted data should go first into a known data type to ensure its format is as expected. once thats done you can do what you like with it.
jollyJumper
April 29th, 2007 5:40pm
Actually, they just shipped me "Writing Secure Code with Windows Vista" where they talk about that very thing. Basically I think the process is that if a function is common to three or more security holes it gets added to the banned list, as long as there is a suitable replacement.
And I tell you, I understand so much more now that I'm on the inside of MS.
strncpy and memcpy are perfectly safe functions to use, and extremely valuable. The only danger is if the programmer insists on copying from unknown sources, or into locations that aren't appropriate. They do make certain implications that only a fool doesn't follow:
1. You know the size of what you're copying from.
2. You know the size of what you're copying to.
3. You have allocated appropriate space in the destination.
That said, the OpenBSD team did replace strncpy with strlcpy, which is just slightly more paranoid that strncpy.
> strncpy and memcpy are perfectly safe functions to use
You do realize the "if" is what makes these perfectly unsafe to use?
son of parnas
April 30th, 2007 10:52am