From 1df6a97e6a72f060b83199cf7499970568213dce Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Sat, 28 Jun 2008 08:52:56 +0000 Subject: [PATCH] 2008-06-28 10:44 UTC+0200 Viktor Szakats (harbour.01 syenar hu) * tests/extend2.c * source/debug/dbgentry.c * contrib/hbodbc/odbc.c * contrib/hbw32/tprinter.c * contrib/hbziparch/hbziparc.c * contrib/hbziparch/hbzipnew.cpp * contrib/hbpgsql/postgres.c ! Changed strcpy() -> hb_strncpy() ! Changed strcat() -> hb_strncat() ! Some possible buffer overruns fixed along the way in hbziparch.lib ! Fixed some filename buffer sizes in hbziparch.lib ; TOFIX: There are still some remaining strcpy()/strcat() calls in Harbour code: core: dbgentry.c, hbwince.c contrib: hbnf, hbw32, hbwhat32, hbtip foreign code: zlib, sqlite2/3 Not all of these are necessarily bugs (but it's difficult to know without checking each). --- harbour/ChangeLog | 20 ++++++++++++ harbour/contrib/hbodbc/odbc.c | 20 ++++++------ harbour/contrib/hbpgsql/postgres.c | 38 +++++++++++------------ harbour/contrib/hbw32/tprinter.c | 26 +++++++--------- harbour/contrib/hbziparch/hbziparc.c | 42 +++++++++++++------------- harbour/contrib/hbziparch/hbzipnew.cpp | 6 ++-- harbour/source/debug/dbgentry.c | 11 ++++--- harbour/tests/extend2.c | 4 +-- 8 files changed, 93 insertions(+), 74 deletions(-) diff --git a/harbour/ChangeLog b/harbour/ChangeLog index 977ff3cc88..bd3abe82b7 100644 --- a/harbour/ChangeLog +++ b/harbour/ChangeLog @@ -8,6 +8,26 @@ 2008-12-31 13:59 UTC+0100 Foo Bar */ +2008-06-28 10:44 UTC+0200 Viktor Szakats (harbour.01 syenar hu) + * tests/extend2.c + * source/debug/dbgentry.c + * contrib/hbodbc/odbc.c + * contrib/hbw32/tprinter.c + * contrib/hbziparch/hbziparc.c + * contrib/hbziparch/hbzipnew.cpp + * contrib/hbpgsql/postgres.c + ! Changed strcpy() -> hb_strncpy() + ! Changed strcat() -> hb_strncat() + ! Some possible buffer overruns fixed along the way in hbziparch.lib + ! Fixed some filename buffer sizes in hbziparch.lib + ; TOFIX: There are still some remaining strcpy()/strcat() + calls in Harbour code: + core: dbgentry.c, hbwince.c + contrib: hbnf, hbw32, hbwhat32, hbtip + foreign code: zlib, sqlite2/3 + Not all of these are necessarily bugs (but it's + difficult to know without checking each). + 2008-06-28 08:53 UTC+0200 Viktor Szakats (harbour.01 syenar hu) * include/set.ch * include/hbset.h diff --git a/harbour/contrib/hbodbc/odbc.c b/harbour/contrib/hbodbc/odbc.c index 72d2e38c0b..f842a6a053 100644 --- a/harbour/contrib/hbodbc/odbc.c +++ b/harbour/contrib/hbodbc/odbc.c @@ -240,7 +240,7 @@ HB_FUNC( SQLFETCH ) /* HB_SQLFETCH( hStmt ) --> nRetCode */ HB_FUNC( SQLGETDATA ) /* HB_SQLGETDATA( hStmt, nField, nType, nLen, @cBuffer ) --> nRetCode */ { - SQLLEN lLen, lInitBuff; + SQLLEN lLen, lInitBuff, lBuffLen; PTR bBuffer, bOut; WORD wType, wResult; int iReallocs = 0; @@ -254,7 +254,7 @@ HB_FUNC( SQLGETDATA ) /* HB_SQLGETDATA( hStmt, nField, nType, nLen, @cBuffer ) - wResult = ! SQL_NO_DATA; while( wResult != SQL_NO_DATA ) { - wResult = SQLGetData( ( HSTMT ) hb_parnl( 1 ), hb_parni( 2 ), wType, ( PTR ) bBuffer, lLen, &lLen ); + wResult = SQLGetData( ( HSTMT ) hb_parnl( 1 ), hb_parni( 2 ), wType, ( PTR ) bBuffer, lLen, &lLen ); if( wResult == SQL_SUCCESS && iReallocs == 0 ) { hb_storclen( ( LPSTR ) bBuffer, ( ULONG ) ( lLen < 0 ? 0 : ( lLen < hb_parnl( 4 ) ? lLen : hb_parnl( 4 ) ) ), 5 ); @@ -266,22 +266,22 @@ HB_FUNC( SQLGETDATA ) /* HB_SQLGETDATA( hStmt, nField, nType, nLen, @cBuffer ) - if( lLen >= lInitBuff ) { /* data right truncated! */ - bOut = ( char * ) hb_xgrab( (ULONG) lLen + 1 ); - hb_strncpy( (char *) bOut, (char *) bBuffer, lLen ); - lLen = lLen - lInitBuff+2; - bBuffer = ( char * ) hb_xrealloc( bBuffer, (ULONG) lLen ); + lBuffLen = lLen; + bOut = ( char * ) hb_xgrab( ( ULONG ) lBuffLen + 1 ); + hb_strncpy( ( char * ) bOut, ( char * ) bBuffer, lLen ); + lLen = lLen - lInitBuff + 2; + bBuffer = ( char * ) hb_xrealloc( bBuffer, ( ULONG ) lLen ); + iReallocs++; } else { hb_storclen( ( LPSTR ) bBuffer, ( ULONG ) ( lLen < 0 ? 0 : ( lLen < hb_parnl( 4 ) ? lLen : hb_parnl( 4 ) ) ), 5 ); break; } - iReallocs++; } - else if( (wResult == SQL_SUCCESS || wResult == SQL_SUCCESS_WITH_INFO ) && iReallocs > 0 ) + else if( ( wResult == SQL_SUCCESS || wResult == SQL_SUCCESS_WITH_INFO ) && iReallocs > 0 ) { - /* TOFIX: Possible buffer overrun. Shouldn't we rather use memcpy()? */ - strcat( (char*) bOut, (char *) bBuffer ); + hb_strncat( ( char * ) bOut, ( char * ) bBuffer, lBuffLen ); hb_storclen( ( LPSTR ) bOut, ( ULONG ) ( lLen + lInitBuff - 1 ), 5 ); wResult = SQL_SUCCESS; break; diff --git a/harbour/contrib/hbpgsql/postgres.c b/harbour/contrib/hbpgsql/postgres.c index 0e335d946b..22526c8efd 100644 --- a/harbour/contrib/hbpgsql/postgres.c +++ b/harbour/contrib/hbpgsql/postgres.c @@ -375,87 +375,87 @@ HB_FUNC( PQMETADATA ) case BITOID: if( typemod >= 0 ) length = ( int ) typemod; - strcpy( buf, "bit" ); + hb_strncpy( buf, "bit", sizeof( buf ) - 1 ); break; case BOOLOID: length = 1; - strcpy( buf, "boolean" ); + hb_strncpy( buf, "boolean", sizeof( buf ) - 1 ); break; case BPCHAROID: if( typemod >= 0 ) length = ( int ) ( typemod - VARHDRSZ ); - strcpy( buf, "character" ); + hb_strncpy( buf, "character", sizeof( buf ) - 1 ); break; case FLOAT4OID: - strcpy( buf, "real" ); + hb_strncpy( buf, "real", sizeof( buf ) - 1 ); break; case FLOAT8OID: - strcpy( buf, "double precision" ); + hb_strncpy( buf, "double precision", sizeof( buf ) - 1 ); break; case INT2OID: - strcpy( buf, "smallint" ); + hb_strncpy( buf, "smallint", sizeof( buf ) - 1 ); break; case INT4OID: - strcpy( buf, "integer" ); + hb_strncpy( buf, "integer", sizeof( buf ) - 1 ); break; case OIDOID: - strcpy( buf, "bigint" ); + hb_strncpy( buf, "bigint", sizeof( buf ) - 1 ); break; case INT8OID: - strcpy( buf, "bigint" ); + hb_strncpy( buf, "bigint", sizeof( buf ) - 1 ); break; case NUMERICOID: length = ( ( typemod - VARHDRSZ ) >> 16 ) & 0xffff; decimal = ( typemod - VARHDRSZ ) & 0xffff; - strcpy( buf, "numeric" ); + hb_strncpy( buf, "numeric", sizeof( buf ) - 1 ); break; case DATEOID: - strcpy( buf, "date" ); + hb_strncpy( buf, "date", sizeof( buf ) - 1 ); break; case TIMEOID: case TIMETZOID: - strcpy( buf, "timezone" ); + hb_strncpy( buf, "timezone", sizeof( buf ) - 1 ); break; case TIMESTAMPOID: case TIMESTAMPTZOID: - strcpy( buf, "timestamp" ); + hb_strncpy( buf, "timestamp", sizeof( buf ) - 1 ); break; case VARBITOID: if( typemod >= 0 ) length = (int) typemod; - strcpy( buf, "bit varying" ); + hb_strncpy( buf, "bit varying", sizeof( buf ) - 1 ); break; case VARCHAROID: if( typemod >= 0 ) length = ( int ) ( typemod - VARHDRSZ ); - strcpy( buf, "character varying" ); + hb_strncpy( buf, "character varying", sizeof( buf ) - 1 ); break; case TEXTOID: - strcpy(buf, "text"); + hb_strncpy( buf, "text", sizeof( buf ) - 1 ); break; case CASHOID: - strcpy( buf, "money" ); + hb_strncpy( buf, "money", sizeof( buf ) - 1 ); break; default: - strcpy( buf, "not supported" ); - break; + hb_strncpy( buf, "not supported", sizeof( buf ) - 1 ); + break; } pField = hb_arrayGetItemPtr( pResult, i + 1 ); diff --git a/harbour/contrib/hbw32/tprinter.c b/harbour/contrib/hbw32/tprinter.c index f44705a897..9c31b9ceef 100644 --- a/harbour/contrib/hbw32/tprinter.c +++ b/harbour/contrib/hbw32/tprinter.c @@ -55,21 +55,17 @@ #if defined(HB_OS_WIN_32) && \ !( defined(__RSXNT__) || defined(__CYGWIN__) || defined(HB_WINCE) ) -# include +#include -# if defined(__LCC__) -# include -# endif +#if defined(__LCC__) +# include +#endif -# define HB_OS_WIN_32_USED -# include "hbapi.h" -# include "hbapiitm.h" +#define HB_OS_WIN_32_USED +#include "hbapi.h" +#include "hbapiitm.h" -BOOL hb_GetDefaultPrinter( char * pPrinterName, LPDWORD pdwBufferSize ); -BOOL hb_GetPrinterNameByPort( char * pPrinterName, LPDWORD pdwBufferSize, char * pPortName, - BOOL bSubStr ); - -# define MAXBUFFERSIZE 255 +#define MAXBUFFERSIZE 255 BOOL hb_isLegacyDevice( LPSTR pPrinterName ) { @@ -292,7 +288,7 @@ BOOL hb_GetPrinterNameByPort( char * pPrinterName, LPDWORD pdwBufferSize, char * szPrinterName = HB_TCHAR_CONVFROM( pPrinterEnum->pPrinterName ); if( *pdwBufferSize >= strlen( szPrinterName ) + 1 ) { - strcpy( pPrinterName, szPrinterName ); + hb_strncpy( pPrinterName, szPrinterName, *pdwBufferSize ); Result = TRUE; } /* Store name length + \0 char for return */ @@ -308,8 +304,8 @@ BOOL hb_GetPrinterNameByPort( char * pPrinterName, LPDWORD pdwBufferSize, HB_FUNC( PRINTERPORTTONAME ) { - char szDefaultPrinter[MAXBUFFERSIZE]; - DWORD pdwBufferSize = MAXBUFFERSIZE; + char szDefaultPrinter[ MAXBUFFERSIZE ]; + DWORD pdwBufferSize = sizeof( szDefaultPrinter ); if( ISCHAR( 1 ) && hb_parclen( 1 ) > 0 && hb_GetPrinterNameByPort( szDefaultPrinter, &pdwBufferSize, hb_parcx( 1 ), diff --git a/harbour/contrib/hbziparch/hbziparc.c b/harbour/contrib/hbziparch/hbziparc.c index ad69f8200f..b8707090b5 100644 --- a/harbour/contrib/hbziparch/hbziparc.c +++ b/harbour/contrib/hbziparch/hbziparc.c @@ -123,7 +123,7 @@ static void UnzipCreateArray( char *szSkleton, int uiOption) PHB_ITEM Temp; BOOL bOkAdd; int ulLen = hb_arrayLen(hbza_ZipArray); - char sRegEx[ _POSIX_PATH_MAX + _POSIX_PATH_MAX ]; + char sRegEx[ _POSIX_PATH_MAX + _POSIX_PATH_MAX + 1 ]; for ( ul = 0 ; ul < ulLen; ul ++ ) { @@ -531,7 +531,7 @@ HB_FUNC( HB_ZIPFILE ) if ( pParam ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; PHB_ITEM pExclude = hb_param( 10, HB_IT_STRING | HB_IT_ARRAY ); BYTE *pCurDir; char *szZipFileName; @@ -567,9 +567,9 @@ HB_FUNC( HB_ZIPFILE ) if ( ! strchr( hb_parc( 1 ), OS_PATH_DELIMITER ) ) { - strcpy( szFile, (char *) pCurDir ); - strcat( szFile, OS_PATH_DELIMITER_STRING) ; - strcat( szFile, hb_parc( 1 ) ) ; + hb_strncpy( szFile, (char *) pCurDir, sizeof( szFile ) - 1 ); + hb_strncat( szFile, OS_PATH_DELIMITER_STRING, sizeof( szFile ) - 1 ); + hb_strncat( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); } else hb_strncpy( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); @@ -605,7 +605,7 @@ HB_FUNC( HB_GETFILESINZIP ) { if( ISCHAR( 1 ) ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; char *szZipFileName; PHB_ITEM pArray; @@ -633,7 +633,7 @@ HB_FUNC( HB_GETFILECOUNT ) if( ISCHAR( 1 ) ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; char * szZipFileName; hb_strncpy( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); @@ -754,7 +754,7 @@ HB_FUNC( HB_ZIPFILEBYTDSPAN ) if ( pParam ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; PHB_ITEM pExclude = hb_param( 11, HB_IT_STRING | HB_IT_ARRAY ); char *szZipFileName; BYTE *pCurDir; @@ -781,9 +781,9 @@ HB_FUNC( HB_ZIPFILEBYTDSPAN ) */ if ( ! strchr( szFile, OS_PATH_DELIMITER ) ) { - strcpy( szFile, (char *) pCurDir ); - strcat( szFile, OS_PATH_DELIMITER_STRING) ; - strcat( szFile, hb_parc( 1 ) ) ; + hb_strncpy( szFile, (char *) pCurDir, sizeof( szFile ) - 1 ); + hb_strncat( szFile, OS_PATH_DELIMITER_STRING, sizeof( szFile ) - 1 ); + hb_strncat( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); } else hb_strncpy( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); @@ -930,7 +930,7 @@ HB_FUNC( HB_ZIPFILEBYPKSPAN ) if ( pParam ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; PHB_ITEM pExclude = hb_param( 10, HB_IT_STRING | HB_IT_ARRAY ); char *szZipFileName; BYTE * pCurDir ; @@ -954,13 +954,13 @@ HB_FUNC( HB_ZIPFILEBYPKSPAN ) hb_fsChDir( pCurDir ) ; /* by JGS, wait until adding the directory to the file name if not specified hb_xfree( pCurDir ) ; - strcpy( szFile, hb_parc( 1 ) ); + hb_strncpy( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); */ if ( ! strchr( szFile, OS_PATH_DELIMITER ) ) { - strcpy( szFile, (char *) pCurDir ); - strcat( szFile, OS_PATH_DELIMITER_STRING) ; - strcat( szFile, hb_parc( 1 ) ) ; + hb_strncpy( szFile, (char *) pCurDir, sizeof( szFile ) - 1 ); + hb_strncat( szFile, OS_PATH_DELIMITER_STRING, sizeof( szFile ) - 1 ); + hb_strncat( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); } else hb_strncpy( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); @@ -1067,7 +1067,7 @@ HB_FUNC( HB_UNZIPFILE ) if( ISCHAR( 1 ) && ( ISARRAY( 6 ) || ISCHAR( 6 ) ) ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; PHB_ITEM pUnzip = hb_param( 6, HB_IT_ANY ); char *szZipFileName; BYTE *pCurDir; @@ -1223,7 +1223,7 @@ HB_FUNC( HB_ZIPDELETEFILES ) if ( pDelZip ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; char *szZipFileName; int ulLen; @@ -1349,7 +1349,7 @@ HB_FUNC( HB_ZIPDELETEFILES ) HB_FUNC( HB_ZIPTESTPK ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; char *szZipFileName; hb_strncpy( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); @@ -1552,7 +1552,7 @@ HB_FUNC( HB_UNZIPFILEINDEX ) if( pDelZip ) { - char szFile[ _POSIX_PATH_MAX ]; + char szFile[ _POSIX_PATH_MAX + 1 ]; PHB_ITEM Temp,DelZip; char* szZipFileName; int ulLen; @@ -1645,7 +1645,7 @@ HB_FUNC(HB_UNZIPALLFILE) { if ( ! ISCHAR(6) && ! ISARRAY(6) ) { - char szFile[_POSIX_PATH_MAX]; + char szFile[ _POSIX_PATH_MAX + 1 ]; char *szZipFile; PHB_ITEM pProgress = ISBLOCK( 7 ) ? hb_itemNew( hb_param( 7, HB_IT_BLOCK ) ) : hb_itemNew( NULL ); hb_strncpy( szFile, hb_parc( 1 ), sizeof( szFile ) - 1 ); diff --git a/harbour/contrib/hbziparch/hbzipnew.cpp b/harbour/contrib/hbziparch/hbzipnew.cpp index abe64e6f9b..cfd201d0c0 100644 --- a/harbour/contrib/hbziparch/hbzipnew.cpp +++ b/harbour/contrib/hbziparch/hbzipnew.cpp @@ -749,7 +749,7 @@ int hb_UnzipSel( char *szFile, PHB_ITEM pBlock, BOOL lWithPath, char *szPassWord } else { - strcpy(szPath,pbyBuffer); + hb_strncpy( szPath, pbyBuffer, _POSIX_PATH_MAX ); } hb_fsChDir((BYTE*)"\\"); @@ -862,7 +862,7 @@ void hb_SetZipComment( char *szComment ) { int iLen = strlen( ( const char * ) szComment ) + 1; hbza_pZipI.szComment = ( char* ) hb_xgrab( iLen ); - strcpy( hbza_pZipI.szComment, szComment ); + hb_strncpy( hbza_pZipI.szComment, szComment, iLen - 1 ); } void hb_SetZipReadOnly(int iRead ) @@ -1151,7 +1151,7 @@ int hb_UnzipAll(char *szFile,PHB_ITEM pBlock, BOOL bWithPath,char *szPassWord,ch } else { - strcpy(szPath,pbyBuffer); + hb_strncpy( szPath, pbyBuffer, _POSIX_PATH_MAX ); } hb_fsChDir((BYTE*)"\\"); diff --git a/harbour/source/debug/dbgentry.c b/harbour/source/debug/dbgentry.c index 1bced51f90..44a9f419a9 100644 --- a/harbour/source/debug/dbgentry.c +++ b/harbour/source/debug/dbgentry.c @@ -1043,6 +1043,7 @@ hb_dbgEvalMakeBlock( HB_WATCHPOINT *watch ) PHB_ITEM pBlock; BOOL bAfterId = FALSE; char *s; + int buffsize; watch->nVars = 0; while ( watch->szExpr[ i ] ) @@ -1180,10 +1181,12 @@ hb_dbgEvalMakeBlock( HB_WATCHPOINT *watch ) i++; } - s = ( char * ) ALLOC( 8 + strlen( watch->szExpr ) + 1 + 1 ); - strcpy( s, "{|__dbg|" ); - strcat( s, watch->szExpr ); - strcat( s, "}" ); + buffsize = 8 + strlen( watch->szExpr ) + 1; + + s = ( char * ) ALLOC( buffsize + 1 ); + hb_strncpy( s, "{|__dbg|", buffsize ); + hb_strncat( s, watch->szExpr, buffsize ); + hb_strncat( s, "}", buffsize ); pBlock = hb_itemNew( NULL ); if( ! hb_dbgEvalMacro( s, pBlock ) ) diff --git a/harbour/tests/extend2.c b/harbour/tests/extend2.c index 7785e94ca6..802f64c284 100644 --- a/harbour/tests/extend2.c +++ b/harbour/tests/extend2.c @@ -185,7 +185,7 @@ CLIPPER HB_UNDOC2() char szText[ 25 ]; _retc( "Hello word" ); - strcpy( szText, _parc( -1 ) ); + hb_strncpy( szText, _parc( -1 ), sizeof( szText ) - 1 ); szText[ 5 ] = 0; _retc( szText ); } @@ -201,7 +201,7 @@ CLIPPER HB_UNDOC4() char szText[ 25 ]; _retds( _pards( 1 ) ); - strcpy( szText, _pards( -1 ) ); + hb_strncpy( szText, _pards( -1 ), sizeof( szText ) - 1 ); szText[ 3 ] = '1'; _retds( szText ); }