Security Enhanced CRT, Safer Than Standard Library?

In a blog post Danny Kalev published earlier this year on InformIT, an example was presented demonstrating how one could write valid but insecure code involving vector and auto_ptr which compiles without any warning, despite other warnings Micorsoft’s recent compilers would’ve issued against standard compliant code. Along with other points he made in the post, Danny suggests Micorsoft doesn’t really care about your code safety. I couldn’t have agreed with him more, and would like to contribute my own analysis (aka. my two cents) in support of Danny’s finding.


Most of us are very well aware of the often-exploited security vulnerability in strcpy, and Microsoft’s answer to it is strcpy_s, which is said to be a more secure alternative that takes an additional parameter, numberOfElements as they called it:

char *strcpy(
   char *strDestination,
   const char *strSource 
);
errno_t strcpy_s(
   char *strDestination,
   size_t numberOfElements,
   const char *strSource 
);

But wait, doesn’t strcpy_s look awfully similar to another standard function?

char *strncpy(
   char *strDest,
   const char *strSource,
   size_t count
);

Put aside the subtle differences in types of return value and parameter naming/ordering, and the insignificant behavioral difference (in regard to buffer overruns, which only applies when things start to screw up) when numberOfElements/count is too small. They are effectively the same.

Yes, I get the semantical differentiation Micorsoft has been trying to sell us programmers. But I failed to understand how is strcpy_s more secure than strncpy syntactically.

Wouldn’t it make more sense if compiler vendors would educate programmers (a.k.a. their customers) the habit of using strncpy instead of strcpy, and use count as the size of the memory buffer strDest points to, for security reasons. But, no, Microsoft had to tell programmers to embrace their proprietary Security Enhanced CRT functions and hints, in a security note, that we can’t use strncpy safely:

strncpy does not check for sufficient space in strDest; it is therefore a potential cause of buffer overruns. Keep in mind that count limits the number of characters copied; it is not a limit on the size of strDest.

If the parameter count couldn’t buy strncpy security, I just don’t see how strcpy_s could have made the cut with numberOfElements.

Leave a Reply


SPEAK YOUR MIND

16 thoughts on “Security Enhanced CRT, Safer Than Standard Library?

  1. 半路

    除了使用 strcpy() 的習慣早就應該改為 strncpy() 以外,
    就連 fopen()、sprintf()、fscanf() 等等也會跳出討厭的 warnings。

    使用這些 _s 版本的函式真的會比較安全嗎?

  2. fr3@K Post author

    我的天哪~~ 連 fopen 都有對應的所謂安全版本 – fopen_s.

    雖然寫了這篇文字來幹樵 Microsoft 的 Security Enhancements in the CRT, 但也只是針對幾個 function 來做分析而已. 並沒有特地去檢驗整個列表 (Security Enhanced CRT functions), 更沒有預期到會有這麼誇張的情形.

    檢視 MSDN 對 fopen_s 的說明才注意到, 它比 fopen 安全 的地方竟然是在檢查 parameter 是否為 NULL!!! (以回傳 EINVAL 來表示這錯誤狀況) 這.. 這.. 這真是讓我大吃一驚. 我不認為這個思維是對的, 傳遞 NULL 給 fopen/fopen_s 根本就是 logic error (programmer 耍白爛), 不是 runtime error 啊~~ Library implementation 有用 assert 在 debug mode 給個 core dump 就很夠意思了, 即便是 undefined behavior 也只能說是 programmer 自己找的.

    先往好的地方想, Microsoft 的這個做法也不見得那麼突兀, 如果拿來跟它力推 C++/CLI (以前叫作 managed C++) 這件事擺在一起, 也可算是一貫的思維. 我願意給 Microsoft 這點 benefit of the doubt, 如果有 strlen_s 的話.

    畢竟 strlen 該是最常被用的 string function 了吧, 怎麼可能沒有 strlen_s? 怎麼可以容忍 strlen 是這麼的不安全, 連 NULL 也不檢查. 你說是吧!?

    PS. 又多看了一下, 如果設了一個 no-op 的 invalid parameter handler, fopen 的行為就跟 fopen_s 一模一樣了.

    [Update: Sep 25th, 2008 at 23:40]
    一查之下, 還真的曾經一度有 strlen_s 這玩意的存在. 但後來又被 deprecate, 以 strnlen 取代.

    這不是自打耳光嗎? strncpy 有什麼不好一定要 deprecate? strcpy_s 又有什麼強處是用 strncpy 搞不定的?

    [Update: Sep 25th, 2008 at 23:55]
    其實 strlen_s 還在! (只是 deprecate), 但也不過是一個為了維護 source code backward compatibility 而 轉 call strnlen 的 wrapper.

  3. mikeb

    Probably the main problem with strncpy() – which is a well known problem – is that is might not actually null terminate the destination buffer. Of course this problem can be addressed by the programmer (often by slapping a ” at the end of the buffer after the strncpy() call or something similar). However, it’s *very* common to see uses of strncpy() that don’t properly handle this problem.

    strcpy_s() always terminates the destination buffer (if the size is not 0).

    Another problem (but not from a security context) is that strncpy() will always write to the entire destination buffer (to zero fill it if necessary). This can be a performance issue if the destination buffer is large and the source string is short.

  4. Tinfoil

    Someone has put some tinfoil-head comments in the Wikipedia strcpy article, and linked to this site. So, even though it’s old, I’ll add a comment.

    I don’t think Microsoft will ever win with the tinfoil heads. If they fix the problems with strncpy that make it bug-prone (as mentioned by mikeb), they will be accused of breaking the standard. If they introduce a version that works properly (which as far as I can tell strncpy_s may be), they will be accused of trying to lock developers into their compiler.

    Even worse than the tinfoil heads are the standards fundamentalists, who think standards were handed down by the programming gods, so that anything in a standard must be good, anything not in it must be bad, and any attempt to change it is heresy. They are oblivious to the fact that standards are typically derived from imperfect implementations designed in particular environments for particular jobs, which happened to catch on, but can often be improved.

    The truth is that strcpy and strncpy are poorly designed functions, which were created at a time when technology and coding practices were far less developed than they are today. If they didn’t exist, no competent programmer in the 21st century would ever invent such bug-promoting and inefficient interfaces. I applaud Microsoft for trying to fix the dilapidated pile of rubbish that is the C library, but I suspect the standards fundamentalists will fight to the death against this unspeakable heresy.

  5. fr3@K Post author

    Hi Tinfoil,

    I do agree with you that the standard is not a godsend and strcpy families are not as good as they could be, but that does not make strcpy deprecated. Think pointer casting, C-style pointer casting is unsafe. (unless you know what you are doing, the same also applies to strcpy/strncpy) The last time I checked, Microsoft’s compiler didn’t warn me that they were deprecated.

    Without a compiler warning from Microsoft’s compiler, do we consider GetSystemDirectory() (just a random function that copies strings) “safe”? I guess most would think. If the return code from the function is checked and error is handled, then yes, it is. But what is the difference between asking programmers to check return code for error and pad/overwrite the last char? My take, not that much. Programmers check for error because they’ve been educated to do so, and they could also be educated to pad/overwrite the last char with '\0'.

    The thing is, IMHO, educating us programmers, especially C/C++ programmers who often write low level code that manipulates memory directly, of writing secure code is THE only thing that would work. Supplementing a supposedly safer library is all well, but education is much more. So that the systems we build would be less vulnerable facing threats that we know and threats that we don’t.

    If we take ONLY the better alternative (Microsoft’s) approach, simply deprecates things those are not safe, we may end up deprecate most of C.

  6. Tinfoil2

    You’re really saying that checking for return values is the same as padding/overwriting the last char with null? You got to be joking!

    Checking return values for errors is normal practise. That’s what they are there for, you know. To tell you if there’s an error. It’s a completely different thing to force people to do stuff the library should be doing.

    Some people just prefer to see conspiracies where there are none rather than attempts to make things better.

    (And I won’t even start to mention what kind of crap gcc lets go through and VC++ doesn’t, unless you specify -ansi and -pedantic and this and that)

  7. fr3@K Post author

    Checking return values for errors is normal practise.

    Yes, it’s perfectly trivial and normal, and why do we do it? Because we were told to, and we were explained why.

    And don’t get me wrong, I do like Microsoft’s C++ compilers, from version 7.1 and on anyhow.

    Allow me to elaborate my point using an analogy. Say a car maker installs a new gadget in their cars so that their cars detect drunk drivers, so when a car detects a drunk person in driver’s seat, it won’t start. This is all good. Nevertheless, with or without this gadget, we need to educate/ask drivers only drive with their sanity. In hope that drivers would not drive when they are on something high.

    And this post is not really about Microsoft, I just ranted at this one compiler vendor, who fail to sell me their secure programming strategy.

    If Security Enhanced CRT is all it takes for you to program securely, fine by me.

  8. Tinfoil

    Hi fr3@K,

    I think strcpy et al should be deprecated by any sensible programming guidelines, and the standard C library ought to offer better designed versions. I’ll note that the OpenBSD documentation for strcpy/strncpy recommends using strlcpy instead, effectively deprecating strncpy on OpenBSD. (Do the tinfoil heads think OpenBSD developers are trying to ‘lock in’ developers too?) It isn’t simply a matter of being ‘unsafe’, it’s the fact that by usually but not always null-terminating, and forcing developers to handle the special cases, these poorly designed routines promote bugs. They are amongst the worst offenders in the C library.

    As a simple example, consider copying a string of some length to a new heap buffer (“size_t length; char *source; char *destination;”). Some programmers may habitually work with string lengths (“length = strlen(source);”), and add space for the null character during allocation (“destination = malloc((length + 1) * sizeof(*destination)); strncpy(destination, source, length)”; destination[length] = ‘\0′;), whilst others work with the buffer length (“length = strlen(input) + 1″), and subtract during string operations (“destination = malloc(length * sizeof(*destination)); strncpy(destination, source, length – 1); destination[length - 1] = ”;”.

    On most development projects, multiple developers work together. If coding practices for using strcpy et al aren’t agreed in advance, you could very well end up with code that mixes the two conventions noted above, along with other practices. Even if all are properly written, it is entirely possible that one of the developers (or a third developer) might at some point, under time pressure, work on unfamiliar code and use the wrong convention for an allocation or a copy. Doing so could very easily introduce bugs where the code seemingly works (copies the full string), but is in fact corrupting the next heap block, with the added possibility of the null character being overwritten later on, and buffer overflows. Crucially, mistakes of this kind do not result in any error codes that can be checked.

    The need to modernise the bug-promoting string functions in the C library was highlighted by OpenBSD’s developers in the 90s, and OpenBSD has offered strlcpy since 1998. Both OpenBSD and Visual C++ provide simple alternatives to the bug-promoting standard versions, which allow for easy detection of errors, and thereby make bugs of the above kind far less likely to occur.

    It’s a sad reflection on the glacial pace of standards processes and the strength of ideologically driven zealots that the obsolete string routines in the C library have not yet been officially deprecated. I think Microsoft’s strcpy_s et al look even better than the OpenBSD solutions, but the zealots will fight heresy from both OpenBSD and Microsoft until their last breaths.

    As a quick comment on your car analogy, I’d say that we should all learn to drive safely (which obviously precludes drink-driving), but that doesn’t mean seat belts and airbags ought to be opposed because they ‘encourage people to drive unsafely’, and a ‘motor standard’ based on an implementation from circa 1950 would not include either.

      ed: fixed ‘\0′ – fr3@K
  9. Tinfoil

    Apparently my ‘\’s disappeared in the above comment. I suppose the backslash has to be doubled.

  10. Tinfoil

    Now in that last comment, the nought after the backslash disappeared!

  11. Bill Spitzak

    The correct replacement is strlcpy as used by BSD. Both Microsoft and glibc are being assholes about this by not putting it in their systems.

    To the poster attempting to defend Microsoft: BSD did not make their compiler throw errors when you call strncpy and say you should rewrite your code to call the BSD-only strlcpy. And as the original article said, Microsoft is producing a bogus warning about strncpy when in fact it is equally as “safe” as their strcpy_s. They also complain about _snprintf which is IDENTICAL to their snprintf_s! If it is not a conspiracy, it is an indication that their programmers are idiots. Not sure which one you prefer!

  12. Tinfoil

    Bill Spitzak,

    First, Microsoft’s compiler issues a warning, not an error. By default, warnings do not prevent builds completing, and it is exceeding common for gcc to spew an enormous number of warnings when building widely used projects. Moreover, any competent developer who uses Microsoft’s compiler with the highest warning level (where warnings are reported as errors) knows how to selectively disable individual warnings, either in specific source files/headers, or globally in project-wide headers. Anyone who has chosen non-default compiler settings that introduce this ‘problem’ and can’t fix it in a matter of seconds by disabling the warning in a project-wide header simply doesn’t know enough about Microsoft’s compiler to be considered competent to use it.

    Second, your claim that strncpy is as ‘safe’ as strcpy_s is patently ridiculous. Even if correctly used, strncpy does not guarantee that the destination string is null-terminated, and nor does it offer any error reporting mechanism if the source string exceeds the count. Both strcpy_s and strlcpy fix these problems, and are quite obviously ‘safer’ than strncpy. It’s incredible that anyone could seriously claim that strncpy is as ‘safe’ as strcpy_s (or strlcpy).

    Third, although you proclaim BSD’s strlcpy is ‘correct’, Microsoft’s strcpy_s is arguably better for two reasons. The first is that it is designed for the common case where truncation of the source string is not the intended behaviour. As a result, it fails if the destination buffer is too small, rather than copying a truncated string. (Where truncation is the intended behaviour, Microsoft’s strncpy_s allows this to be explicitly specified.) Moreover, returning an error code makes error detection easier, and allows a custom error handler to be installed (which, for example, can halt execution).

    Your comment regarding _snprintf and _snprintf_s reveals yet another misunderstanding on your part. The buffer-size argument to _snprintf_s allows a custom error handler to be invoked in cases where the buffer is too small to hold the output string. It also makes the interface consistent with the other ‘*_s’ routines, and provides a mechanism for the developer to explicitly request truncation. To say that _snprintf_s is identical with _snprintf is simply nonsense, rooted in ignorance.

    As a rule, it is best to avoid criticising something until you first understand what it is that you are criticising. Given that Microsoft’s ‘*_s’ routines are certainly better than the standard C library routines, and arguably better than the BSD ‘l’ routines, your claim of either incompetence or a conspiracy looks rather silly. Moreover, if it were some nefarious ‘lock-in’ conspiracy, why would Microsoft be pushing for their ‘*_s’ routines to be be added to the C standard?

    My guess as to why Microsoft decided to deprecate the ‘unsafe’ C library routines is that bugs introduced by careless use of these routines are responsible for a large number of security vulnerabilities found in software that runs on Microsoft’s Windows platform (and on other platforms, but that’s a separate matter). Bugs of this kind undoubtedly impose both financial and reputational costs on users of Microsoft’s C compiler and the OSes that host it. They also arguably impose reputational costs on Microsoft, even when they are blameless. Reducing these costs to Microsoft and their customers is certainly reason enough to deprecate the ‘unsafe’ string routines in the C library in favour of the ‘safe’ ones.

  13. fr3@K Post author

    Hi Tinfoil,

    First of all, let me just say that I am glad that you stopped calling people “tinfoil heads” in your response to Spitzak’s comment.

    The scenario you laid out does demonstrate your points very well, (BTW, I fixed the ‘\0′ thing in your comment) but still, it doesn’t mean one can’t code securely using strncpy.

    This post is NOT about how I hated a set of (supposedly) more secured string functions that is not in the Standard, but rather, to dispute one compiler vendor’s secure programming strategy, which IMO, produces more marketing effects than secure coding minds.

    Apology to all if I’ve been misleading. By implying strncpy is as safe as strcpy_s, I meant one can’t overrun destination buffers, as a result of invocations of either strncpy or strcpy_s, when used correctly. And one could indeed code securely, with strncpy. (though additional effort may be required) As the zero filling behavior of strncpy, which wastes CPU cycles, is another topic.

  14. Tinfoil

    Hi fre3@K,

    I still think the claim of a Microsoft conspiracy to ‘lock in’ developers cannot be taken seriously, except by those who could easily be convinced to wear tinfoil on their heads. It would be quite a stupid strategy to spend time and money trying to get these ‘*_s’ routines standardised if ‘lock in’ were Microsoft’s goal.

    Second, the issue is not whether a routine is inherently ‘unsafe’ — virtually anything /can/ be used ‘safely’ — but whether an interface is so poorly designed that it promotes ‘unsafe’ coding. That is, it cannot be used ‘safely’ without a lot of extra code to check things that the library routine itself could (and should) very easily check.

    Even the original strcpy can be used perfectly safely if developers keep track of string lengths and buffer sizes. The reason strcpy is ‘unsafe’ is because (a) the interface does not make it clear that developers /have to/ think about these things themselves, and (b) the compiler/runtime does not offer any checking or error reporting, so developers have to rely on spotting bugs in code reviews, with debug allocators (for heap buffers) or from crash dumps.

    The main point of these ‘*_s’ routines (well, my guess as to the point) is to force developers to think about string lengths, buffer sizes and truncation whenever they call one of the library routines, and to help them spot cases where they have made mistakes. If you don’t know the string lengths, buffer sizes and whether or not you want to truncate, then your code is already wrong. If you do, then switching to these ‘*_s’ routines is trivial. These are not marketing effects. Thinking about these things is exactly what every developer using the C library should do. Unfortunately, many of them don’t, and even the ones that do sometimes make mistakes, for which the standard C library interfaces/runtime offer no help.

    On the whole, strcpy is an awful design and strncpy makes things worse, by introducing confusing special cases without even guaranteeing null termination. strlcpy is a good (but non-standard) solution, and so is strcpy_s. Where strcpy_s wins is in requiring that developers who want truncation explicitly say so, in its more powerful error-handling mechanisms and in the overall consistency of the ‘*_s’ interfaces.

  15. Bill Spitzak

    Just want to point out to mr tinfoil that strlcpy does indicate if truncation happened, by returning a value larger than the value passed to it. This value is actually useful in that it is the size the buffer must be reallocated to.

  16. Bill Spitzak

    It would be quite a stupid strategy to spend time and money trying to get these ‘*_s’ routines standardised if ‘lock in’ were Microsoft’s goal.

    No, it is a brilliant strategy, because it allows you to make claims like this in order to try to defend their actions. It has zero effect on the “lock in” (the funcitons are trivial enough that no “standard” is needed to reverse-engineer them) but it provides a nice-sounding excuse for their compiler to spew warnings about portable code.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>