MTurner100646 (Community Member) asked a question.

Could someone explain why this c code produces a Stack-based overflow flaw?

TCHAR* ULibLongString( TCHAR* pszOutput, ushort idx0, ushort idx1, uchar idx2base )

//-----------------------------------------------------------------------------

{

TCHAR *pszText;

 

  if ( pszOutput == NULL )

   pszText = allocate_mem( 257*2 );

  else

   pszText = pszOutput;

  

  StringCchPrintf( pszText, 257, L"%s%s%s%s", 

   ULibGetString( idx0, idx1, idx2base ), ULibGetString( idx0, idx1, (uchar)(idx2base+1) ),

   ULibGetString( idx0, idx1, (uchar)(idx2base+2) ), ULibGetString( idx0, idx1, (uchar)(idx2base+3) ) );

  return pszText;

}

 

In ALL but one call to this function the pszOutput parameter is passed as a 257 TCHAR array. In the other case NULL is passed so invokes the allocate_mem function, which is essentailly a wrapper to malloc()

 

The Veracode flaw description says:

"This call to StringCchPrintfW() contains a buffer overflow. The source string has an allocated size of (unavailable) bytes from the result of format string "%%s%%s%%s%%s", and the destination buffer is 64 bytes. If an attacker can control the data written into the buffer, the overflow may result in execution of arbitrary code."

 

Thanks


  • Hi @MTurner100646 (Community Member)​ ,

     

    Please consider checking if ULibLongString is DLLExported.

    If this function is DLL exported Veracode Static Analysis will no longer consider it's usages in the application but will consider the size of the input '(unavailable)' and will report a flaw of CWE 121 Stack Based Buffer Overflow if this is used with dangerous functions such as StringCchPrintf.

     

    In this case we would recommend manually reviewing usages of this function and if all usages fit inside the destination buffer documenting this in a mitigation proposal: https://docs.veracode.com/r/improve_mitigation .

     

    If you have any thoughts for improving this CWE, please consider creating a Veracode Idea at https://community.veracode.com/s/ideas .

     

    Thank you,

    Boy Baukema

    Expand Post
    • MTurner100646 (Community Member)

      Hi @Boy, Security Consultant (Veracode)​ 

       

      ULibLongString is not a DLL export - this code is from an exe. The function ULibGetString is imported from a static library and will return a TCHAR array of up to 64 TCHARs hence the size of the destination buffer. What I don't understand is why Veracode thinks our destination buffer is 64 Bytes. Should we be using a function other than StringCchPrintf?

       

      Thanks

      Martin

      Expand Post

Topics (4)

No articles found
Loading

Ask the Community

Get answers, share a use case, discuss your favorite features, or get input from the community.