Please Join me in welcoming memcpy() to the SDL Rogues Gallery

Please Join me in welcoming memcpy() to the SDL Rogues Gallery

Rate This
  • Comments 11

Over the last few years I have written a number of articles, papers and books describing some of the dangers of using various buffer-manipulating C runtime functions. Well-known examples of bad function calls include strcpy(), strcat(), strncpy(), strncat(), gets() and their foul brethren. The SDL bans these and many other functions with dubious security history. But, when it comes to banning functions, we must tread a very fine line because we can’t just ban something because it looks odd, or that gut instinct tells us it’s bad, we can only ban functionality that has been demonstrated to cause security vulnerabilities and only if there is a viable alternative.

Because we have seen many security vulnerabilities in products from Microsoft and many others, including ISVs and competitors, and because we have a viable replacement, I am “proud” to announce that we intend to add memcpy() will to the SDL C and C++ banned API list later this year as we make further revisions to the SDL. Right now, memcpy() is on the SDL Recommended banned list, but will soon be added to the SDL banned API requirement list now that we have more feedback from Microsoft product groups.

The following security updates all have one thing in common: memcpy().

·         MS03-030 (DirectX)

·         MS03-043 (Messenger Service)

·         MS03-044 (Help and Support)

·         MS05-039 (PnP)

·         MS04-011 (PCT)

·         MS05-030 (Outlook Express)

·         CVE-2007-3999 (MIT Kerberos v5)

·         CVE-2007-4000 (MIT Kerberos v5)

·         …many more!

It’s not just memcpy() that we’re banning; we will also ban CopyMemory() and RtlCopyMemory(), and the replacement function is memcpy_s().

Banning memcpy() in your code

You too should start banning memcpy() in your new code, here’s what you can do right now:

Add the following line of code to a common header file:

#pragma deprecated (memcpy, RtlCopyMemory, CopyMemory)

Every time the compiler sees an instance of the banned functions, you’ll get the following warning:

warning C4995: 'memcpy': name was marked as #pragma deprecated

In Visual C++, you can also add this early in a common header:

#define _CRT_SECURE_WARNINGS_MEMORY

And you will get warnings like:

warning C4996: 'memcpy': This function or variable may be unsafe. Consider using memcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. c:\program files\microsoft visual studio 9.0\vc\include\wchar.h(1201) : see declaration of 'memcpy'

You can deprecate these functions if you’re using gcc by poisoning them:

#pragma GCC poison memcpy RtlCopyMemory CopyMemory

Fixing memcpy() calls

Thankfully, it’s pretty simple to migrate a call to memcpy() to a safer call to memcpy_s(); the big difference is memcpy_s() takes one extra parameter: the size of the destination buffer. If nothing else, memcpy_s makes you think about the size of the target buffer.

The SAL-decorated function signature in VC++ 2008 is:

errno_t __cdecl
    memcpy_s(
        _Out_opt_bytecap_post_bytecount_(_DstSize, _MaxCount)
               
void * _Dst,
        _In_ rsize_t _DstSize,
        _In_opt_bytecount_(_MaxCount) const void * _Src,
        _In_ rsize_t _MaxCount
    );

All you need to do is update calls to memcpy() by adding the size of the destination buffer. So calls like this:

char dst[32];

memcpy(dst,src,len);

becomes

char dst[32];

memcpy_s(dst,sizeof(dst), src,len);

Of course, you can easily make a call to memcpy_s() insecure by getting the buffer sizes wrong. The following code is no better than memcpy():

memcpy_s(dst,len, src,len);

You’ve been warned!

I wonder when Larry, Steve and Linus will start banning strcpy() in their products?

Comments
  • PingBack from http://microsoft-sharepoint.simplynetdev.com/please-join-me-in-welcoming-memcpy-to-the-sdl-rogues-gallery/

  • Linus is not the one who would make a decision on memcpy: it would be the maintainers of the GNU C library ("glibc"). They might well add a memcpy_s macro (undoubtably there is already a secure memcpy function in GNU libc, but with a different name) but they would almost certainly not remove or forbid access to the C standard memcpy.

  • For those writing in C, you will be interested in a posting on The Security Development Lifecycle blog

  • Are we ever going to see Microsoft Ada? It can do everything C does, but in a much safer way.

  • Questionably useful.  If you're using C but can afford pointless checks, then you're using the wrong language.  (Well, I think that C is the wrong language on anything with a filesystem, but that's a separate issue.)

    Sure, this sounds good, but I'm not convinced memcpy_s will really help.  It's only checking consistency between 2 of the arguments, which means that all 4 can still be wrong.  Not something I'd call "secure".  (At very least I'd want checks that src+srcsize <= dst || dst+dstsize <= src, but what you really to check need is that the memory ranges are valid subranges of allocated arrays, which you can't without either ruining performance or breaking code.)

    I wonder whether Larry, Steve and Linus will actually notice a difference in the quality of Steve (Ballmer)'s products...

  • Functions like memcpy_s don't make code more secure and are poorly written and will degrade performance of your code as much as 3 times. What will be your next step - deprecate pointers?

    This is just a publicity stunt. Microsoft has to stop letting marketing people write code.

  • A.M.

    absolutely no-one from mktg touched the content of this article. this is a technical article, describing some real changes we're making to our dev processes. you should take a look at http://www.microsoft.com/sdl to see it's not mktg.

  • I do not believe this approach – that of requiring source and target buffer lengths to be supplied as function arguments – solves the problem! It simply introduces another opportunity for error.

    This approach only works if the lengths supplied are correct, and I think it is easy for a programmer to get this wrong while having a false sense of security.

  • I have always used something like this...

    #define BUFF_SIZE 32

    char dst[BUFF_SIZE], src[BUFF_SIZE];

    memcpy(dst,src,BUFF_SIZE);

    First thing that I do is to assign the buffer length to a variable, and use it for both source and destination while calling malloc(), memcpy(), etc.... I follow similar descipline while using strxxx() and it has worked fine. I have tried using strxxx_S() versions found it to be redundent.

    Word like "poison" is uncalled for because problem is with programmer, and as you say, a poor programmer could drive even memcpy_s() to ditch :-).  

  • You realize that replacing memcpy with a secure version (say memcpy_s) would probably add overhead. This might not be significant in say a word processor, but for performance critical application (like simulation, games, video, database) this overhead is probably going to be felt specially when the functions going to be called millions of times for small blocks of data.

    Second, you also realize that the source of the problem here is an ignorant programmer, NOT the programming language. modifying a programming language like that for the sake of forcing the programmer to do well is NOT historically a feature of the C/C++ language. May be Java yes. C is usually meant to be as flexible as possible.

    Third, you know that in modern OSes (Windows too), each process has it's own address space that is completely isolated from all other processes. This is supported by hardware, built in features in processors. So there NO WAY a memcpy can touch any other address outside it's own process (i.e the programmers program). Thus, all security issues are confined within that process of this ignorant programmer.

    If you want to ban functions like that, then you might as well just ban the inline assembler keyword __asm. Which can do much more serious havoc if used inappropriately.

  • Seriously, memcpy?

    Since this article was written, the proprietary and no more secure "memcpy_s"  has not gained any traction at all  The fact that a number of far more secure OSs continue to use memcpy should be evidence of this error.

    Ok, I get that at the time MS was getting blamed hard for security issues.  But those were created by proprietary cruft in the first place.  This just repeated history.  Nothing was made more secure.  Only more of a mess for no gain.

    Given the results, why should anyone incorporate this non-standard variant into their code?  It taints a codebase and makes it incompatible with all other platforms, and has never made Windows more secure.  Frankly, seems like a better idea to keep the code base pure and let the Windows users just use the web version.

    This was a mistake that never accomplished its goals and only undermined the Windows platform.  It's 2014, I think it's time to get back on board the standards train.

    Wish MS would've used the resources to make useful vector and numerics and thread pooling frameworks instead.

Page 1 of 1 (11 items)
Leave a Comment
  • Please add 7 and 8 and type the answer here:
  • Post