Search

_snprintf_s causes array out of bounds write when the _TRUNCATE flag is used for count by mattnewport

Closed
as By Design Help for as By Design

0
0
Sign in
 to vote
Type: Bug
ID: 289013
Opened: 7/25/2007 10:39:21 AM
Access Restriction: Public
2
Workaround(s)
2
User(s) can reproduce this bug
According to the documentation for _snprintf_s(), if _TRUNCATE is passed as the count parameter then the output string should be truncated to the length of the buffer and a null terminator appended. However, when _TRUNCATE is passed as the count parameter the function actually writes a null terminator 2 bytes before the beginning of the destination string. This array out of bounds access triggers the stack corruption detection when basic runtime checks are enabled.
Details (expand)
Product Language
English

Version

Visual Studio 2005 (All Products and Editions) Service Pack 1 w/ Windows Vista Update
Operating System
Windows XP Professional
Operating System Language
English
Steps to Reproduce
The following code demonstrates the bug:

#include <cstdio>
#include <cstring>

int _tmain(int /*argc*/, _TCHAR* /*argv*/[])
{
static const int bufLength = 256;
char temp1[bufLength] = { 0 };
strcpy_s(temp1, "A string.");
char temp2[bufLength] = { 0 };
printf_s("Stack at (temp2 - 4) before call to _snprintf_s(): %x\n", *((unsigned int*)(temp2 - 4)));
_snprintf_s(temp2, _TRUNCATE, "This call writes out of bounds.");
printf_s("Stack at (temp2 - 4) after call to _snprintf_s(): %x\n", *((unsigned int*)(temp2 - 4)));
printf_s("temp2 = \"%s\"\n", temp2);
return 0;
}
Actual Results
The output when this is run with Basic Runtime Checks enabled is:

Stack at (temp2 - 4) before call to _snprintf_s(): cccccccc
Stack at (temp2 - 4) after call to _snprintf_s(): cc00cccc
temp2 = "This call writes out of bounds."

And this error is triggered:
"Run-Time Check Failure #2 - Stack around the variable 'temp2' was corrupted."
Expected Results
The stack before the beginning of temp2 should not be written to and the basic runtime check should not be triggered.
TAP Code (if applicable)
 
      You can indicate your satisfaction with how Microsoft handled this issue by completing this quick 3 question survey. [Details]

 

File Attachments
File Name Submitted By Submitted On File Size  
snprintf_s_bug.zip (restricted) mattnewport 7/25/2007 -
Sign in to post a comment.
Posted by gwbush on 12/30/2008 at 8:58 AM
Could someone explain what the design is? What is the logic that makes this behavior acceptable?
Posted by jaybus on 1/25/2008 at 3:37 AM
I located the bug in stdio.h line 307:

__DEFINE_CPP_OVERLOAD_SECURE_FUNC_0_2_ARGLIST(int, _snprintf_s, _vsnprintf_s, __out_bcount(_Size) char, _Dest, __in size_t, _Size, __in_z __format_string const char *,_Format)

It must be:

__DEFINE_CPP_OVERLOAD_SECURE_FUNC_0_2_ARGLIST(int, _snprintf_s, _vsnprintf_s, __out_bcount(_Size) char, _Dest, __in size_t, _Count, __in_z __format_string const char *,_Format)

The second _Size in the original version causes the problem, because the template parameter and the second function parameter get mixed up.

A test with the modified version showed no more problems.
Posted by RyanMillikin on 12/4/2007 at 2:58 PM
I have experienced this problem in VS2005 using the _snprintf_s overload that accepts a reference to an array (3 explicit argument version).

I have found two suitable workarounds for this. The most simple would be to manually pass the size and use the other overload for this call.
The second would be to fix the error in the header where the call is. This entails going into stdio.h and finding the call, or simply creating a call to the overloaded version with a reference to an array as the first argument and right clicking on that to go to the definition. Once there, you can change the 6th parameter that is _Size to something else like _Count.

This problem is caused by the parameter for the truncated size having the same variable name as the template variable. This causes both the size of the array in bytes and the amount you would like to truncate to both being the same value that you pass in. _TRUNCATE evaluates to 0xFFFFFFFF, which will allow the function to write past the end of the buffer and result in stack corruption.
Posted by Robert Johnson on 9/18/2007 at 8:20 AM
This certainly is a bug and SHOULD NOT be classed as 'Closed (By Design)'. It is not limited to using _TRUNCATE either.

I received an exception with this, which I couldn't believe:

char dateBuf[11];
_snprintf_s( dateBuf, 10, "%02d/%02d/%04d", 1, 2, 3 );

I want to know why Microsoft have 'designed' it this way, it's wrong.

My advice is to use snprintf instead. Obviously these safe versions don't work properly.

Posted by Claus Brod on 9/17/2007 at 3:22 AM
We found the same issue. The problem does not occur when using the Unicode version (_snwprintf_s), but can be reproduced reliably in ANSI builds. Since we always compile in /RTCsu mode, the stack corruption is automatically detected at runtime. Sample code:

bool foobar(void)
{
char buffer[16];
_snprintf_s(buffer, _TRUNCATE, "foobar");
return (0 == strcmp(buffer, "foobar"));
}

Posted by mattnewport on 8/2/2007 at 4:19 PM
I'd still like an explanation for why this is not considered a bug. Either the documentation is wrong or the function has a bug. This feedback system seems highly flawed when bugs can be marked resolved or closed without any comment or explanation. With our bug tracking system you can't change the status of a bug without offering some kind of explanation for the status change.
Posted by mattnewport on 7/26/2007 at 3:42 PM
Why has this been marked 'Resolved (By Design)'? It's causing an out of bounds array write when used in accordance with the documentation, that doesn't seem like acceptable behaviour for what is supposed to be a 'secure' variant of a standard CRT function. It's worth noting that the behaviour is also different from the non templated version of _snprintf_s() which takes a buffer size and a count and which does not have this bug.
Posted by Microsoft on 7/25/2007 at 6:39 PM
Thanks for your feedback. We have reproduced this bug on Visual Studio 2005 SP1, and we are sending this bug to the appropriate group within the VisualStudio Product Team for triage and resolution.

Thank you,
Visual Studio Product Team.
Posted by Microsoft on 7/25/2007 at 5:19 PM
Thank you for your feedback. We are currently investigating. If this issue is urgent, please call support directly (see http://support.microsoft.com).

Thank you,
Visual Studio Product Team
Sign in to post a workaround.
Posted by Robert Johnson on 9/18/2007 at 8:21 AM
Use snprintf instead of snprintf_s
Posted by RyanMillikin on 12/4/2007 at 2:56 PM
I have experienced this problem in VS2005 using the _snprintf_s overload that accepts a reference to an array (3 explicit argument version).

I have found two suitable workarounds for this. The most simple would be to manually pass the size and use the other overload for this call.
The second would be to fix the error in the header where the call is. This entails going into stdio.h and finding the call, or simply creating a call to the overloaded version with a reference to an array as the first argument and right clicking on that to go to the definition. Once there, you can change the 6th parameter that is _Size to something else like _Count.

This problem is caused by the parameter for the truncated size having the same variable name as the template variable. This causes both the size of the array in bytes and the amount you would like to truncate to both being the same value that you pass in. _TRUNCATE evaluates to 0xFFFFFFFF, which will allow the function to write past the end of the buffer and result in stack corruption.
© 2012 Microsoft Corporation. All rights reserved. Microsoft Connect Terms of Use | Trademarks | Privacy Statement