Udostępnij za pośrednictwem

Good News! strlen isn’t a banned API after all.

  • Artykuł

We were doing some code reviews on the new Win7 SDK samples the other day and one of the code reviewers noticed that the code used wcslen to compute the length of a string.

He pointed out that the SDL Banned API page calls out strlen/wcslen as being banned APIs:

For critical functions, such as those accepting anonymous Internet connections, strlen must also be replaced:

Table 19. Banned string length functions and replacements

Banned APIs StrSafe Replacement Safe CRT Replacement
strlen, wcslen, _mbslen, _mbstrlen, StrLen, lstrlen String*Length strnlen_s

I was quite surprised to see this, since I’m not aware of any issues where the use of strlen/wcslen could cause security bugs.

 

I asked Michael Howard about this and his response was that Table 19 has a typo – the word “server” is missing in the text, it should be “For critical serverfunctions, such as those accepting anonymous Internet connections, strlen must also be replaced”. 

Adding that one word makes all the difference.  And it makes sense – if you’re a server and accepting anonymous data over the internet, an attacker could cause you to crash by issuing a non null terminated string that was long enough – banning the API forces the developer to think about the length of the string.

Somewhat OT, but I also think that the table is poorly formatted – the “For critical…” text should be AFTER the table title – the way the text is written, it appears to be a part of the previous section instead of being attached as explanatory text on Table 19 (but that’s just the editor in me).

 

Apparently in SDL v5.0 (which hasn’t yet shipped) the *len functions are removed from the banned API list entirely.

Comments

  • Anonymous
    June 23, 2009
    It makes perfect sense that mbslen is banned. Too easy to misuse number of MB characters as the string length. And if there is a malformed multi-byte character, who knows if mbslen won't barf. You can hope. strlen can turn bad, if you happen to use strncpy which doesn't always nul-terminate the buffer.

  • Anonymous
    June 23, 2009
    Good point Alex - mbslen IS one of those icky APIs.  But the others aren't. And strlen isn't the problem with the strlen/strncpy usage - it's strncpy which is correctly on the list of banned APIs.

  • Anonymous
    June 23, 2009
    The comment has been removed

  • Anonymous
    June 23, 2009
    The comment has been removed

  • Anonymous
    June 23, 2009
    OT, but ... Larry, welcome back! You've been missed. I'm sure you're busy with Windows 7 but I'm looking forward to more posts. You're one of a select few Microsoft blogs I read regularly.

  • Anonymous
    June 25, 2009
    OT, but the table itself is fixed width, causing it to be truncated.  There is seriously no reason for it to be fixed width in this case.   Make fit to the width of the browser so that the contents aren't truncated. :)