From 472c3592ed6e4dbb04250608af87b2b540e4fe74 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Thu, 7 Jan 2010 17:17:24 +0000 Subject: [PATCH] 2010-01-07 18:16 UTC+0100 Viktor Szakats (harbour.01 syenar.hu) * src/vm/extrap.c * contrib/hbwin/win_prn3.c + Added TOFIXes for unsafe function usage: IsBadWritePtr(), IsBadReadPtr(), lstrlen(), lstrcpy(), lstrcat() ; QUESTION: Any idea how to fix that? win_prn3.c can probably be fixed with some good general coding idea. * contrib/hbwin/wce_smsc.c ! Fixed to not use unsafe CRTL functions. * src/rtl/oemansi.c ! Rewritten HB_OEMTOANSI() and HB_ANSITOOEM() to not use Windows API functions marked as unsafe. New version is a bit slower but won't mark Harbour apps as unsafe in an audit. ; Please test and review. * utils/hbmk2/hbmk2.prg * config/wce/msvcarm.mk * config/win/msvc.mk % Deleted -Gs MSVC option. Testing with MSVC 2008 I've found this have no effect on x86 builds, and it makes x64 builds slighly less efficient by forcing stack checks in each function call. This seems to contradict MSDN, which doesn't suggest such difference between x86 and x64: http://msdn.microsoft.com/en-us/library/9598wk25.aspx % Deleted -GS- MSVC option used for wce targets for MSVC >= 8.00. '-GS-' will disable stack cookies (on by default), thus trading app size/speed for security. Maybe this is preferred for wce users, but in Harbour I'd rather opt to pass this decision to users for all targets. Enable with: HB_USER_CFLAGS=-GS- / -cflag={allmsvc}-GS- MSDN: http://msdn.microsoft.com/en-us/library/8dbf701c.aspx --- harbour/ChangeLog | 36 ++++++++++++++++++++++++++++ harbour/config/wce/msvcarm.mk | 2 +- harbour/config/win/msvc.mk | 2 +- harbour/contrib/hbwin/wce_smsc.c | 19 +++++++-------- harbour/contrib/hbwin/win_prn3.c | 9 +++++-- harbour/src/rtl/oemansi.c | 40 +++++++++++++++++++++++++------- harbour/src/vm/extrap.c | 4 ++++ harbour/utils/hbmk2/hbmk2.prg | 4 ++-- 8 files changed, 91 insertions(+), 25 deletions(-) diff --git a/harbour/ChangeLog b/harbour/ChangeLog index 9c5c2e9d6a..a343bac485 100644 --- a/harbour/ChangeLog +++ b/harbour/ChangeLog @@ -17,6 +17,42 @@ past entries belonging to author(s): Viktor Szakats. */ +2010-01-07 18:16 UTC+0100 Viktor Szakats (harbour.01 syenar.hu) + * src/vm/extrap.c + * contrib/hbwin/win_prn3.c + + Added TOFIXes for unsafe function usage: + IsBadWritePtr(), IsBadReadPtr(), lstrlen(), lstrcpy(), lstrcat() + ; QUESTION: Any idea how to fix that? win_prn3.c can + probably be fixed with some good general coding + idea. + + * contrib/hbwin/wce_smsc.c + ! Fixed to not use unsafe CRTL functions. + + * src/rtl/oemansi.c + ! Rewritten HB_OEMTOANSI() and HB_ANSITOOEM() to not use + Windows API functions marked as unsafe. New version is a bit + slower but won't mark Harbour apps as unsafe in an audit. + ; Please test and review. + + * utils/hbmk2/hbmk2.prg + * config/wce/msvcarm.mk + * config/win/msvc.mk + % Deleted -Gs MSVC option. Testing with MSVC 2008 I've found + this have no effect on x86 builds, and it makes x64 builds + slighly less efficient by forcing stack checks in each function + call. This seems to contradict MSDN, which doesn't suggest such + difference between x86 and x64: + http://msdn.microsoft.com/en-us/library/9598wk25.aspx + % Deleted -GS- MSVC option used for wce targets for MSVC >= 8.00. + '-GS-' will disable stack cookies (on by default), thus trading + app size/speed for security. Maybe this is preferred for wce + users, but in Harbour I'd rather opt to pass this decision to users + for all targets. + Enable with: HB_USER_CFLAGS=-GS- / -cflag={allmsvc}-GS- + MSDN: + http://msdn.microsoft.com/en-us/library/8dbf701c.aspx + 2010-01-07 08:58 UTC-0800 Pritpal Bedi (pritpal@vouchcac.com) * contrib/hbide/hbide.prg * contrib/hbide/ideactions.prg diff --git a/harbour/config/wce/msvcarm.mk b/harbour/config/wce/msvcarm.mk index 9a8ec19f03..0ac64ab935 100644 --- a/harbour/config/wce/msvcarm.mk +++ b/harbour/config/wce/msvcarm.mk @@ -50,7 +50,7 @@ ifneq ($(HB_BUILD_OPTIM),no) ifneq ($(filter $(HB_COMPILER_VER),600 700 710),) CFLAGS += -Oxsb1 -EHsc -YX -GF else - CFLAGS += -Od -Os -Gy -GS- -EHsc- + CFLAGS += -Od -Os -Gy -EHsc- endif endif diff --git a/harbour/config/win/msvc.mk b/harbour/config/win/msvc.mk index 03b0f4f9e2..462cd9149d 100644 --- a/harbour/config/win/msvc.mk +++ b/harbour/config/win/msvc.mk @@ -22,7 +22,7 @@ CC_OUT := -Fo CFLAGS += -I. -I$(HB_INC_COMPILE) -CFLAGS += -nologo -Gs +CFLAGS += -nologo ifeq ($(HB_BUILD_MODE),c) CFLAGS += -TC diff --git a/harbour/contrib/hbwin/wce_smsc.c b/harbour/contrib/hbwin/wce_smsc.c index 1a40b45425..a2f357350e 100644 --- a/harbour/contrib/hbwin/wce_smsc.c +++ b/harbour/contrib/hbwin/wce_smsc.c @@ -69,40 +69,39 @@ HB_FUNC( WCE_SMSSENDMESSAGE ) /* cMessage, cNumber */ if( hr == ERROR_SUCCESS ) { + SMS_ADDRESS smsaDestination; + void * hMessage; void * hPhoneNumber; + HB_SIZE nMessageLen; HB_SIZE nPhoneNumberLen; - LPCTSTR sztMessage = HB_PARSTRDEF( 1, &hMessage , NULL ); + LPCTSTR sztMessage = HB_PARSTRDEF( 1, &hMessage , &nMessageLen ); LPCTSTR sztPhoneNumber = HB_PARSTRDEF( 2, &hPhoneNumber, &nPhoneNumberLen ); - if( nPhoneNumberLen <= SMS_MAX_ADDRESS_LENGTH ) + if( nPhoneNumberLen <= HB_SIZEOFARRAY( smsaDestination.ptsAddress ) ) { - SMS_ADDRESS smsaDestination; TEXT_PROVIDER_SPECIFIC_DATA tpsd; SMS_MESSAGE_ID smsmidMessageID = 0; /* Create the destination address */ memset( &smsaDestination, 0, sizeof( smsaDestination ) ); smsaDestination.smsatAddressType = ( *sztPhoneNumber == _T( '+' ) ) ? SMSAT_INTERNATIONAL : SMSAT_NATIONAL; - /* TOFIX: lstrcpy() unsafe and may cause buffer overrun. - Worked around using length check against SMS_MAX_ADDRESS_LENGTH. - [vszakats]. */ - lstrcpy( smsaDestination.ptsAddress, sztPhoneNumber ); - + memcpy( smsaDestination.ptsAddress, sztPhoneNumber, HB_SIZEOFARRAY( smsaDestination.ptsAddress ) ); + /* Set up provider specific data */ tpsd.dwMessageOptions = PS_MESSAGE_OPTION_NONE; tpsd.psMessageClass = PS_MESSAGE_CLASS0; tpsd.psReplaceOption = PSRO_NONE; - + /* Send the message, indicating success or failure */ hb_retnl( SmsSendMessage( smshHandle, NULL, &smsaDestination, NULL, ( PBYTE ) sztMessage, - _tcslen( sztMessage ) * sizeof( wchar_t ), + nMessageLen * sizeof( TCHAR ), ( PBYTE ) &tpsd, 12, SMSDE_OPTIMAL, SMS_OPTION_DELIVERY_NONE, diff --git a/harbour/contrib/hbwin/win_prn3.c b/harbour/contrib/hbwin/win_prn3.c index 0c4a5d1347..d298570ce2 100644 --- a/harbour/contrib/hbwin/win_prn3.c +++ b/harbour/contrib/hbwin/win_prn3.c @@ -204,6 +204,9 @@ static HB_BOOL hb_SetDefaultPrinter( LPCTSTR lpPrinterName ) return HB_FALSE; } + /* TOFIX: Use safe string functions instead of lstrlen(), lstrcpy() and lstrcat(). + [vszakats] */ + /* Allocate buffer big enough for concatenated string. String will be in form "printername,drivername,portname". */ pBuffer = ( LPTSTR ) hb_xgrab( ( lstrlen( lpPrinterName ) + @@ -217,8 +220,10 @@ static HB_BOOL hb_SetDefaultPrinter( LPCTSTR lpPrinterName ) } /* Build string in form "printername,drivername,portname". */ - lstrcpy( pBuffer, lpPrinterName ); lstrcat( pBuffer, TEXT( "," ) ); - lstrcat( pBuffer, ppi2->pDriverName ); lstrcat( pBuffer, TEXT( "," ) ); + lstrcpy( pBuffer, lpPrinterName ); + lstrcat( pBuffer, TEXT( "," ) ); + lstrcat( pBuffer, ppi2->pDriverName ); + lstrcat( pBuffer, TEXT( "," ) ); lstrcat( pBuffer, ppi2->pPortName ); /* Set the default printer in Win.ini and registry. */ diff --git a/harbour/src/rtl/oemansi.c b/harbour/src/rtl/oemansi.c index 65b6035014..82fc26ae15 100644 --- a/harbour/src/rtl/oemansi.c +++ b/harbour/src/rtl/oemansi.c @@ -6,7 +6,7 @@ * Harbour Project source code: * OEM <-> ANSI string conversion functions (Windows specific, Xbase++ ext.) * - * Copyright 1999-2007 Viktor Szakats (harbour.01 syenar.hu) + * Copyright 1999-2010 Viktor Szakats (harbour.01 syenar.hu) * www - http://www.harbour-project.org * * This program is free software; you can redistribute it and/or modify @@ -65,12 +65,23 @@ HB_FUNC( HB_ANSITOOEM ) if( pString ) #if defined( HB_OS_WIN ) { - DWORD ulLen = hb_itemGetCLen( pString ); - char * pszDst = ( char * ) hb_xgrab( ulLen + 1 ); + int nLen = hb_itemGetCLen( pString ); + const char * pszSrc = hb_itemGetCPtr( pString ); - CharToOemBuffA( hb_itemGetCPtr( pString ), pszDst, ulLen ); + int nWideLen = MultiByteToWideChar( CP_ACP, MB_PRECOMPOSED, pszSrc, nLen, NULL, 0 ); + LPWSTR pszWide = ( LPWSTR ) hb_xgrab( ( nWideLen + 1 ) * sizeof( wchar_t ) ); - hb_retclen_buffer( pszDst, ulLen ); + char * pszDst; + + MultiByteToWideChar( CP_ACP, MB_PRECOMPOSED, pszSrc, nLen, pszWide, nWideLen ); + + nLen = WideCharToMultiByte( CP_OEMCP, 0, pszWide, nWideLen, NULL, 0, NULL, NULL ); + pszDst = ( char * ) hb_xgrab( nLen + 1 ); + + WideCharToMultiByte( CP_OEMCP, 0, pszWide, nWideLen, pszDst, nLen, NULL, NULL ); + + hb_xfree( pszWide ); + hb_retclen_buffer( pszDst, nLen ); } #else hb_itemReturn( pString ); @@ -86,12 +97,23 @@ HB_FUNC( HB_OEMTOANSI ) if( pString ) #if defined( HB_OS_WIN ) { - DWORD ulLen = hb_itemGetCLen( pString ); - char * pszDst = ( char * ) hb_xgrab( ulLen + 1 ); + int nLen = hb_itemGetCLen( pString ); + const char * pszSrc = hb_itemGetCPtr( pString ); - OemToCharBuffA( hb_itemGetCPtr( pString ), pszDst, ulLen ); + int nWideLen = MultiByteToWideChar( CP_OEMCP, MB_PRECOMPOSED, pszSrc, nLen, NULL, 0 ); + LPWSTR pszWide = ( LPWSTR ) hb_xgrab( ( nWideLen + 1 ) * sizeof( wchar_t ) ); - hb_retclen_buffer( pszDst, ulLen ); + char * pszDst; + + MultiByteToWideChar( CP_OEMCP, MB_PRECOMPOSED, pszSrc, nLen, pszWide, nWideLen ); + + nLen = WideCharToMultiByte( CP_ACP, 0, pszWide, nWideLen, NULL, 0, NULL, NULL ); + pszDst = ( char * ) hb_xgrab( nLen + 1 ); + + WideCharToMultiByte( CP_ACP, 0, pszWide, nWideLen, pszDst, nLen, NULL, NULL ); + + hb_xfree( pszWide ); + hb_retclen_buffer( pszDst, nLen ); } #else hb_itemReturn( pString ); diff --git a/harbour/src/vm/extrap.c b/harbour/src/vm/extrap.c index c16903534f..9ee51f5fe5 100644 --- a/harbour/src/vm/extrap.c +++ b/harbour/src/vm/extrap.c @@ -171,6 +171,7 @@ LONG WINAPI hb_winExceptionHandler( struct _EXCEPTION_POINTERS * pExceptionInfo pc = ( unsigned char * ) pCtx->Eip; for( i = 0; i < 16; i++ ) { + /* TOFIX: Unsafe funcion. */ if( IsBadReadPtr( pc, 1 ) ) break; hb_snprintf( buf, sizeof( buf ) - 1, " %02X", ( int ) pc[ i ] ); @@ -180,6 +181,7 @@ LONG WINAPI hb_winExceptionHandler( struct _EXCEPTION_POINTERS * pExceptionInfo sc = ( unsigned int * ) pCtx->Esp; for( i = 0; i < 16; i++ ) { + /* TOFIX: Unsafe funcion. */ if( IsBadReadPtr( sc, 4 ) ) break; hb_snprintf( buf, sizeof( buf ), " %08X", sc[ i ] ); @@ -190,10 +192,12 @@ LONG WINAPI hb_winExceptionHandler( struct _EXCEPTION_POINTERS * pExceptionInfo hb_strncat( errmsg, " EIP: EBP: Frame: OldEBP, RetAddr, Params...\n", errmsglen ); eip = pCtx->Eip; ebp = ( unsigned int * ) pCtx->Ebp; + /* TOFIX: Unsafe funcion. */ if( ! IsBadWritePtr( ebp, 8 ) ) { for( i = 0; i < 20; i++ ) { + /* TOFIX: Unsafe funcion. */ if( ( unsigned int ) ebp % 4 != 0 || IsBadWritePtr( ebp, 40 ) || ( unsigned int ) ebp >= ebp[ 0 ] ) break; hb_snprintf( buf, sizeof( buf ), " %08X %08X ", ( int ) eip, ( int ) ebp ); diff --git a/harbour/utils/hbmk2/hbmk2.prg b/harbour/utils/hbmk2/hbmk2.prg index fdb8b817ad..2fdfd7485b 100644 --- a/harbour/utils/hbmk2/hbmk2.prg +++ b/harbour/utils/hbmk2/hbmk2.prg @@ -3057,11 +3057,11 @@ FUNCTION hbmk( aArgs, /* @ */ lPause ) nCmd_Esc := _ESC_DBLQUOTE cOpt_Lib := "-nologo {FA} -out:{OL} {LO}" cOpt_Dyn := "{FD} -dll -out:{OD} {DL} {LO} {LL} {LB} {LS}" - cOpt_CompC := "-nologo -c -Gs" + cOpt_CompC := "-nologo -c" IF hbmk[ _HBMK_lOPTIM ] IF hbmk[ _HBMK_cPLAT ] == "wce" IF nCCompVer >= 800 - cOpt_CompC += " -Od -Os -Gy -GS- -Gm -Zi -GR-" + cOpt_CompC += " -Od -Os -Gy -Gm -Zi -GR-" ELSE cOpt_CompC += " -Oxsb1 -YX -GF" ENDIF