Re: STATUS_STACK_BUFFER_OVERRUN encountered
On Dec 25, 2:22 am, Joseph M. Newcomer <newco...@flounder.com> wrote:
See below...
On Wed, 24 Dec 2008 01:43:01 -0800 (PST), Manoj Jangid <manoj.jan...@gmai=
l.com> wrote:
Hi I am calling a function from MFC dll in C# application.
This function work with C++ application but when I was calling from C#
application terminates unexpectedly.
can anyone tell me why this happening?
I am pasting my source code here
---------------------------------------------------
C# code
--------------
private void button1_Click(object sender, EventArgs e)
{
string strPassWord = textBox1.Text;
string strHash = "";
PasswordHash(strPassWord,ref strHash);
MessageBox.Show(strHash);
}
-----------------
PasswordHash function in my MFC regular statically linked to MFC dll
C++ code
bool __stdcall PasswordHash(LPCTSTR lpPasswor=
d, LPTSTR lpszHash)
{
****
This is REALLY SCARY code. It has an LPTSTR and no explicit length is =
specified, nor is
there any way to determine if the caller actually gave you a buffer that =
is large enough.
This is a MAJOR security hole! This is extremely unsafe code.
****> AFX_MANAGE_STATE(AfxGetStaticModuleState());
CString strPassword(lpPassword);
bool bReturn = false;
HCRYPTPROV hCryptProv;
HCRYPTHASH hHash=NULL;
BYTE pbHash[16];
DWORD dwHashLen= 16;
****
Why use 16 twice? Consider using a #define or static const UINT to def=
ine this and use
the symbolic name
****> DWORD cbContent= strPassword.GetLength() * sizeof(TCHAR);
BYTE* pbContent= (BYTE *) strPassword.GetBuffer(cbContent);
if(CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL,
CRYPT_VERIFYCONTEXT | CRYPT_MACHINE_KEYSET))
{
if(CryptCreateHash(hCryptProv,CALG_MD5,0, 0, &hHash))
{
if(CryptHashData(hHash, pbContent, cbContent, 0))
{
if(CryptGetHashParam(hHash, HP_HASHVAL, pbHash, &dwHashLen, 0))
{
LPTSTR lpTmp = lpszHash;
for (int i = 0; i < 16; i++)
****
Another use of 16. That's three times too many
****>{
const size_t nlen = sizeof(pbHash[i])+2;
****
Why is this sizeof(pbHash[i])? Note that sizeof(pbHash[i]) *always* ha=
s to be 1. And
what's the magic "+2" term for?
_stprintf_s(lpTmp,nlen,_T("%02X"),pbHash[i]);
****
This is even scarier. You are writing to the output buffer, whose leng=
th is unknown, by
claiming that it is big enough. Yet the computation is erroneous. I=
n ANSI, it computes
the nlen term to be 1+2, or 3, but it only needs to be 2; in Unicode apps=
, it computes the
nlen term to be 1+2, or 3, but it needs to be 2 (characters) which is 4 b=
ytes. There's a
lot of confusion here about how to use _stprintf_s. That length is not=
computed based on
what you THINK might be available, but what the input parameters TELL you=
is available.
What input parameter? Well, that's a serious design error. There is=
n't one.>lpTmp += 2;
****
You are advancing the pointer, but where is the bounds check. The corr=
ect code would be
int nlen = Length; // copy parameter to nlen
for(int i = 0; i < MAX_KEY_SIZE; i++)
{
_stprintf_s(lpTmp, nlen, _T("%02X"), pbHash[i]);
--nlen;
lpTmp += 2;
}
This will avoid a buffer overrun. Your code could clobber arbitrary am=
ounts of data (and
don't say "I know the output buffer is big enough" unless you also want t=
o say "and I
understand that secure code never relies on rumors but always passes in a=
length, and
failure to do so can be cause for termination in many companies, but secu=
rity doesn't
matter. I like seeing our name in headlines."
****>}
bReturn = true;
}
}
}
}
****
Have you noticed that code that is not indented is hard to read?
joe
****
CryptDestroyHash(hHash);
CryptReleaseContext(hCryptProv, 0);
return bReturn;
}
-----------------
here problem in my for loop
LPTSTR lpTmp = lpszHash;
for (int i = 0; i < 16; i++)
{
const size_t nlen = sizeof(pbHash[i])+2;
_stprintf_s(lpTmp,nlen,_T("%02X"),pbHash[i]);
lpTmp += 2;
}
//Application will terminates when i=13
whats wrong in this loop I also try this with _stprintf (non secure
version) same problem each time only when I am trying to call in C#
application
--------
Additional Information: The runtime has encountered a fatal error. The
address of the error was at 0x79ee24f1, on thread 0x10c4. The error
code is 0xc0000005. This error may be a bug in the CLR or in the
unsafe or non-verifiable portions of user code. Common sources of this
bug include user marshaling errors for COM-interop or PInvoke, which
may corrupt the stack.
STATUS_STACK_BUFFER_OVERRUN encountered
The thread 'Win32 Thread' (0x1478) has exited with code -1073740791
(0xc0000409).
The thread 'Win32 Thread' (0x854) has exited with code -1073740791
(0xc0000409).
The thread 'Win32 Thread' (0x13e4) has exited with code -1073740791
(0xc0000409).
The program '[6076] CryptClient.exe: Managed' has exited with code
-1073740791 (0xc0000409).
The program '[6076] CryptClient.exe: Native' has exited with code
-1073740791 (0xc0000409).
----------
Kind Regards
Joseph M. Newcomer [MVP]
email: newco...@flounder.com
Web:http://www.flounder.com
MVP Tips:http://www.flounder.com/mvp_tips.htm- Hide quoted text -
- Show quoted text -- Hide quoted text -
- Show quoted text -
Thank you Joseph,
I know code quality is not good, this code is written long back by
some other guy,
I,ll change the code according your suggestion.
Thanks again
Manoj Jangid