From ed3dd0415fa96feeaa9d9a89f2d201fa14accd10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Czerpak?= Date: Wed, 19 Nov 2014 08:37:16 +0100 Subject: [PATCH] 2014-11-19 08:37 UTC+0100 Przemyslaw Czerpak (druzus/at/poczta.onet.pl) * contrib/hbnetio/netiosrv.c * small simplification * src/rtl/hbznet.c ! fixed possible data stream corruption which could be exploited in systems which can accept only one byte in socket write operation when internal TCP output buffers are full. Rather seldom situation but theoretically possible, i.e. in VPN. --- ChangeLog.txt | 10 ++++++++ contrib/hbnetio/netiosrv.c | 48 +++++++++++++++++++------------------- src/rtl/hbznet.c | 34 +++++++++++---------------- 3 files changed, 48 insertions(+), 44 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 2d859c7ba7..7c16acaae8 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -10,6 +10,16 @@ * Change, ! Fix, % Optimization, + Addition, - Removal, ; Comment */ +2014-11-19 08:37 UTC+0100 Przemyslaw Czerpak (druzus/at/poczta.onet.pl) + * contrib/hbnetio/netiosrv.c + * small simplification + + * src/rtl/hbznet.c + ! fixed possible data stream corruption which could be exploited in + systems which can accept only one byte in socket write operation + when internal TCP output buffers are full. Rather seldom situation + but theoretically possible, i.e. in VPN. + 2014-11-18 14:13 UTC+0100 Przemyslaw Czerpak (druzus/at/poczta.onet.pl) * include/dbinfo.ch + added new rddInfo() action: RDDI_DECIMALS diff --git a/contrib/hbnetio/netiosrv.c b/contrib/hbnetio/netiosrv.c index 1968540aa2..4222d9834c 100644 --- a/contrib/hbnetio/netiosrv.c +++ b/contrib/hbnetio/netiosrv.c @@ -356,7 +356,7 @@ static PHB_CONSRV s_consrvNew( HB_SOCKET connsd, const char * szRootPath, HB_BOO return conn; } -static long s_srvRecvAll( PHB_CONSRV conn, void * buffer, long len ) +static HB_BOOL s_srvRecvAll( PHB_CONSRV conn, void * buffer, long len ) { HB_BYTE * ptr = ( HB_BYTE * ) buffer; long lRead = 0, l; @@ -385,10 +385,10 @@ static long s_srvRecvAll( PHB_CONSRV conn, void * buffer, long len ) } } - return lRead; + return lRead == len; } -static long s_srvSendAll( PHB_CONSRV conn, void * buffer, long len ) +static HB_BOOL s_srvSendAll( PHB_CONSRV conn, void * buffer, long len ) { HB_BYTE * ptr = ( HB_BYTE * ) buffer; long lSent = 0, lLast = 1, l; @@ -427,7 +427,7 @@ static long s_srvSendAll( PHB_CONSRV conn, void * buffer, long len ) if( conn->mutex ) hb_threadMutexUnlock( conn->mutex ); } - return lSent; + return lSent == len; } static HB_GARBAGE_FUNC( s_listensd_destructor ) @@ -726,21 +726,21 @@ static HB_BOOL s_netio_login_accept( PHB_CONSRV conn ) { HB_BYTE msgbuf[ NETIO_MSGLEN ]; - if( s_srvRecvAll( conn, msgbuf, NETIO_MSGLEN ) == NETIO_MSGLEN && + if( s_srvRecvAll( conn, msgbuf, NETIO_MSGLEN ) && HB_GET_LE_INT32( msgbuf ) == NETIO_LOGIN ) { long len = HB_GET_LE_INT16( &msgbuf[ 4 ] ); if( len < ( long ) sizeof( msgbuf ) && len == ( long ) strlen( NETIO_LOGINSTRID ) && - s_srvRecvAll( conn, msgbuf, len ) == len ) + s_srvRecvAll( conn, msgbuf, len ) ) { if( memcmp( NETIO_LOGINSTRID, msgbuf, len ) == 0 ) { HB_PUT_LE_UINT32( &msgbuf[ 0 ], NETIO_LOGIN ); HB_PUT_LE_UINT32( &msgbuf[ 4 ], NETIO_CONNECTED ); memset( msgbuf + 8, '\0', NETIO_MSGLEN - 8 ); - if( s_srvSendAll( conn, msgbuf, NETIO_MSGLEN ) == NETIO_MSGLEN ) + if( s_srvSendAll( conn, msgbuf, NETIO_MSGLEN ) ) conn->login = HB_TRUE; } } @@ -792,7 +792,7 @@ HB_FUNC( NETIO_SERVER ) msg = buffer; - if( s_srvRecvAll( conn, msgbuf, NETIO_MSGLEN ) != NETIO_MSGLEN ) + if( ! s_srvRecvAll( conn, msgbuf, NETIO_MSGLEN ) ) break; uiMsg = HB_GET_LE_UINT32( msgbuf ); @@ -811,7 +811,7 @@ HB_FUNC( NETIO_SERVER ) if( size + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -844,7 +844,7 @@ HB_FUNC( NETIO_SERVER ) if( size + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -875,7 +875,7 @@ HB_FUNC( NETIO_SERVER ) if( HB_MAX( size, size2 ) + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( HB_MAX( size, size2 ) + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -885,7 +885,7 @@ HB_FUNC( NETIO_SERVER ) pszDirSpec = pszName ? hb_strdup( pszName ) : NULL; msg[ size2 ] = '\0'; - if( size2 && s_srvRecvAll( conn, msg, size2 ) != size2 ) + if( size2 && ! s_srvRecvAll( conn, msg, size2 ) ) errCode = NETIO_ERR_READ; else if( ! pszName ) errCode = NETIO_ERR_WRONG_FILE_PATH; @@ -939,7 +939,7 @@ HB_FUNC( NETIO_SERVER ) if( size + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -990,7 +990,7 @@ HB_FUNC( NETIO_SERVER ) if( HB_MAX( size, size2 ) + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( HB_MAX( size, size2 ) + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -1001,7 +1001,7 @@ HB_FUNC( NETIO_SERVER ) szOldName = hb_strdup( szFile ); msg[ size2 ] = '\0'; - if( s_srvRecvAll( conn, msg, size2 ) != size2 ) + if( ! s_srvRecvAll( conn, msg, size2 ) ) errCode = NETIO_ERR_READ; else if( ! szOldName ) errCode = NETIO_ERR_WRONG_FILE_PATH; @@ -1038,7 +1038,7 @@ HB_FUNC( NETIO_SERVER ) if( size + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -1072,7 +1072,7 @@ HB_FUNC( NETIO_SERVER ) if( size + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -1107,7 +1107,7 @@ HB_FUNC( NETIO_SERVER ) if( size + conn->rootPathLen >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size + conn->rootPathLen + 1 ); msg[ size ] = '\0'; - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else if( conn->filesCount >= NETIO_FILES_MAX ) errCode = NETIO_ERR_FILES_MAX; @@ -1154,7 +1154,7 @@ HB_FUNC( NETIO_SERVER ) { if( size >= ( long ) ( sizeof( buffer ) - NETIO_MSGLEN ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size + NETIO_MSGLEN ); - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -1273,7 +1273,7 @@ HB_FUNC( NETIO_SERVER ) { if( size >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size ); - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else { @@ -1456,7 +1456,7 @@ HB_FUNC( NETIO_SERVER ) { if( size >= ( long ) sizeof( buffer ) ) ptr = msg = ( HB_BYTE * ) hb_xgrab( size ); - if( s_srvRecvAll( conn, msg, size ) != size ) + if( ! s_srvRecvAll( conn, msg, size ) ) errCode = NETIO_ERR_READ; else if( ! conn->rpc ) errCode = NETIO_ERR_UNSUPPORTED; @@ -1642,7 +1642,7 @@ HB_FUNC( NETIO_SERVER ) else len += NETIO_MSGLEN; - errCode = s_srvSendAll( conn, msg, len ) != len; + errCode = ! s_srvSendAll( conn, msg, len ); if( ptr ) hb_xfree( ptr ); @@ -1690,7 +1690,7 @@ HB_FUNC( NETIO_SRVSENDITEM ) stream = stream->next; } if( stream && stream->type == NETIO_SRVITEM ) - fResult = s_srvSendAll( conn, msg, lLen ) == lLen; + fResult = s_srvSendAll( conn, msg, lLen ); hb_threadMutexUnlock( conn->mutex ); } hb_xfree( msg ); @@ -1730,7 +1730,7 @@ HB_FUNC( NETIO_SRVSENDDATA ) stream = stream->next; } if( stream && stream->type == NETIO_SRVDATA ) - fResult = s_srvSendAll( conn, msg, lLen ) == lLen; + fResult = s_srvSendAll( conn, msg, lLen ); hb_threadMutexUnlock( conn->mutex ); } hb_xfree( msg ); diff --git a/src/rtl/hbznet.c b/src/rtl/hbznet.c index ec09a9c52c..e6943be0fc 100644 --- a/src/rtl/hbznet.c +++ b/src/rtl/hbznet.c @@ -75,7 +75,7 @@ typedef struct _HB_ZNETSTREAM } HB_ZNETSTREAM; -#define HB_ZNET_BUFSIZE 16384 +#define HB_ZNET_BUFSIZE 0x4000 #if MAX_MEM_LEVEL >= 8 # define HB_ZNET_MEM_LEVEL 8 @@ -292,47 +292,41 @@ long hb_znetRead( PHB_ZNETSTREAM pStream, HB_SOCKET sd, void * buffer, long len, static long hb_znetStreamWrite( PHB_ZNETSTREAM pStream, HB_SOCKET sd, HB_MAXINT timeout ) { - long tosnd = HB_ZNET_BUFSIZE - pStream->wr.avail_out; - long snd = 0, rest = 0; + long snd = 0, rest = 0, tosnd; if( pStream->crypt ) { - long size = ( long ) ( pStream->wr.next_out - pStream->crypt_out ); - - if( size > 2 ) + rest = ( long ) ( pStream->wr.next_out - pStream->crypt_out ); + if( rest > 2 ) { - HB_U16 uiLen = ( HB_U16 ) ( size - 2 ); + HB_U16 uiLen = ( HB_U16 ) ( rest - 2 ); HB_PUT_BE_UINT16( pStream->crypt_out, uiLen ); - uiLen = ( HB_U16 ) ( ( ( size + 7 ) ^ 0x07 ) & 0x07 ); - if( ( uInt ) uiLen > pStream->wr.avail_out ) - { - /* it may happen only if encryption was enabled in non empty - * buffer and the unencrypted part has not been flushed yet. - */ - rest = size; - } - else + uiLen = ( HB_U16 ) ( ( ( rest + 0x07 ) ^ 0x07 ) & 0x07 ); + if( ( uInt ) uiLen <= pStream->wr.avail_out ) { while( uiLen-- ) { *pStream->wr.next_out++ = ( Byte ) 0; /* TODO: use better hashing data */ pStream->wr.avail_out--; - size++; + rest++; } /* encrypt the buffer */ - for( tosnd = 0; tosnd < size; tosnd += 8 ) + for( tosnd = 0; tosnd < rest; tosnd += 8 ) hb_znetEncrypt( pStream, pStream->crypt_out + tosnd ); + rest = 0; pStream->crypt_out = pStream->wr.next_out; pStream->wr.next_out += 2; if( pStream->wr.avail_out < 2 ) pStream->skip_out = 2 - pStream->wr.avail_out; pStream->wr.avail_out -= 2 - pStream->skip_out; } - tosnd = ( long ) ( pStream->crypt_out - pStream->outbuf ); } else - tosnd -= 2; + rest = 0; + tosnd = ( long ) ( pStream->crypt_out - pStream->outbuf ); } + else + tosnd = HB_ZNET_BUFSIZE - pStream->wr.avail_out; if( tosnd > 0 ) {