[20879] in bugtraq
Re: Qpopper 4.0.3 **** Fixes Buffer Overflow **** (fwd)
daemon@ATHENA.MIT.EDU (William D. Colburn (aka Schlake))
Tue Jun 5 21:38:35 2001
Date: Tue, 5 Jun 2001 13:51:50 -0600
From: "William D. Colburn (aka Schlake)" <wcolburn@nmt.edu>
To: Roman Drahtmueller <draht@suse.de>
Cc: bugtraq@securityfocus.com, qpopper@qualcomm.com, security@suse.de
Message-ID: <20010605135150.A4213@nmt.edu>
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="x+6KMIRAuhnl3hBn"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <Pine.LNX.4.33.0106051840040.1114-100000@dent.suse.de>; from draht@suse.de on Tue, Jun 05, 2001 at 06:52:23PM +0200
--x+6KMIRAuhnl3hBn
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Here is a patch (attached) to take 4.0.3 down to 4.0.2.
On Tue, Jun 05, 2001 at 06:52:23PM +0200, Roman Drahtmueller wrote:
> We hope that this information is accurate. Version 4.0.2 is not on the ftp
> server any more, and there is no patch from 4.0.2 to 4.0.3.
> We currently feel handicapped in our efforts to check the code for the
> changes wrt the buffer overflow.
>
> SuSE ships qpopper versions 2.53 (with a set of patches that include
> security fixes for this version) for SuSE-6.3, 6.4 and 7.0, SuSE-7.1 and
> the upcoming SuSE-7.2 release have version 3.1.2.
>
> If the above statement is right, then SuSE distributions are not
> vulnerable. However, we wish to double-check such a claim. All kinds of
> verification and transparency are welcome, including an official statement
> from Qualcomm (thanks in advance!).
>
>
> > Changes from 4.0.2 to 4.0.3:
> > ----------------------------
> > 1. Don't call SSL_shutdown unless we tried to negotiate an
> > SSL session. (As suggested by Kenneth Porter.)
> > 2. Fix buffer overflow (reported by Gustavo Viscaino).
>
>
> Thank you,
> Roman Drahtmüller,
> SuSE Security.
> - --
> - -
> | Roman Drahtmüller <draht@suse.de> // "Caution: Cape does |
> SuSE GmbH - Security Phone: // not enable user to fly."
> | Nürnberg, Germany +49-911-740530 // (Batman Costume warning label) |
> - -
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.0.6 (GNU/Linux)
> Comment: http://www.suse.de/
>
> iEYEARECAAYFAjsdDlkACgkQnkDjEAAKq6RVAQCgmAZJGKq6v4J9kjznUy+tlZzm
> j3EAoMyrDlRtE8OgI98T7FN18IfEYfHR
> =G2T2
> -----END PGP SIGNATURE-----
--
William Colburn, "Sysprog" <wcolburn@nmt.edu>
Computer Center, New Mexico Institute of Mining and Technology
http://www.nmt.edu/tcc/ http://www.nmt.edu/~wcolburn
--x+6KMIRAuhnl3hBn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="qpopper.patch"
*** pop_apop.c Fri Jun 1 20:24:33 2001
--- ../../qpopper4.0.2/popper/pop_apop.c Tue Apr 3 18:23:27 2001
***************
*** 205,215 ****
user_name_len = sizeof(p->user) -1;
}
! strlcpy ( p->user, p->pop_parm[1], sizeof(p->user) );
!
! #ifdef SCRAM
! strlcpy ( p->authid, p->user, sizeof(p->authid) ); /* userid is also authentication id */
! #endif /* SCRAM */
pop_log ( p, POP_INFO, HERE, "apop \"%s\"", p->user );
--- 205,212 ----
user_name_len = sizeof(p->user) -1;
}
! strcpy ( p->user, p->pop_parm[1] );
! strcpy ( p->authid, p->user ); /* userid is also authentication id */
pop_log ( p, POP_INFO, HERE, "apop \"%s\"", p->user );
*** pop_config.c Fri Jun 1 20:24:33 2001
--- ../../qpopper4.0.2/popper/pop_config.c Tue Apr 3 18:23:29 2001
***************
*** 5,13 ****
*
* Revisions:
*
- * 06/01/01 [RCG]
- * - Added 'uw-kludge' as synonym for 'uw-kluge'.
- *
* 02/12/01 [RCG]
* - Now saving trace file name.
*
--- 5,10 ----
***************
*** 287,293 ****
{ "update-status-headers" , CfgBool , CfgResNone, kUPDATE_STATUS_HDRS },
{ "user-options" , CfgBool , CfgResUser, kUSEROPTS },
{ "UW-kluge" , CfgBool , CfgResNone, kUW_KLUGE },
- { "UW-kludge" , CfgBool , CfgResNone, kUW_KLUGE },
{ NULL , CfgBad , CfgResNone, LAST_OPT_VAL }
};
--- 284,289 ----
*** pop_dropcopy.c Fri Jun 1 20:24:34 2001
--- ../../qpopper4.0.2/popper/pop_dropcopy.c Tue Apr 3 18:23:30 2001
***************
*** 538,544 ****
}
if ( !p->mmdf_separator ) {
! strlcpy(frombuf, buffer, sizeof(frombuf)); /* Preserve From envelope */
}
/*
--- 538,544 ----
}
if ( !p->mmdf_separator ) {
! strcpy(frombuf, buffer); /* Preserve From envelope */
}
/*
***************
*** 915,921 ****
}
if ( !p->mmdf_separator ) {
! strlcpy ( frombuf, buffer, sizeof(frombuf) );
}
/*
--- 915,921 ----
}
if ( !p->mmdf_separator ) {
! strcpy ( frombuf, buffer );
}
/*
*** pop_parse.c Fri Jun 1 20:24:34 2001
--- ../../qpopper4.0.2/popper/pop_parse.c Tue Apr 3 18:23:34 2001
***************
*** 5,14 ****
*
* Revisions:
*
- * 06/01/01 [rcg]
- * - Fixed empty password treated as empty command
- * (patch submitted by Michael Smith and others).
- *
* 06/20/00 [rcg]
* - Fixed compiler warnings.
*
--- 5,10 ----
***************
*** 99,106 ****
/*
* This is kinda gross. Passwords have to be parsed diffrently
* as they may contain spaces. If you think of a cleaner way,
! * do it. The "p->pop_command[0] == 'p'" is to save a call to
! * strcmp() on every call to pop_parse(); This parsing keeps
* leading and trailing speces behind for the password command.
*/
if ( p->pop_command[0] == 'p' && strcmp(p->pop_command, "pass") == 0 ) {
--- 95,102 ----
/*
* This is kinda gross. Passwords have to be parsed diffrently
* as they may contain spaces. If you think of a cleaner way,
! * do it. The "p->pop_command[0] == 'p'" is so save a call to
! * strcmp() on ever call to pop_parse(); This parsing keeps
* leading and trailing speces behind for the password command.
*/
if ( p->pop_command[0] == 'p' && strcmp(p->pop_command, "pass") == 0 ) {
***************
*** 113,119 ****
}
return ( 1 );
} else
! return ( 0 );
}
}
else {
--- 109,115 ----
}
return ( 1 );
} else
! return ( -1 );
}
}
else {
*** pop_pass.c Fri Jun 1 20:24:35 2001
--- ../../qpopper4.0.2/popper/pop_pass.c Tue Apr 3 18:23:34 2001
***************
*** 12,23 ****
*
* Revisions:
*
! * 06/01/01 [rcg]
! * - Added patch by Clifton Royston to change error message
! * for nonauthfile and authfile tests.
! *
! * 02/03/01 [rcg]
! * - Now logging to p->pop_facility.
*
* 01/30/01 [rcg]
* - Now using p->bDo_timing instead of DO_TIMING.
--- 12,19 ----
*
* Revisions:
*
! * 02/03/01 [RCG]
! * - Now logging to p->pop_facility.
*
* 01/30/01 [rcg]
* - Now using p->bDo_timing instead of DO_TIMING.
***************
*** 1269,1275 ****
*/
if ( p->nonauthfile != NULL && checknonauthfile ( p ) != 0 ) {
sleep ( SLEEP_SECONDS );
! return ( pop_msg ( p, POP_FAILURE, HERE, ERRMSG_AUTH, p->user ) );
}
/*
--- 1265,1271 ----
*/
if ( p->nonauthfile != NULL && checknonauthfile ( p ) != 0 ) {
sleep ( SLEEP_SECONDS );
! return ( pop_msg ( p, POP_FAILURE, HERE, ERRMSG_PW, p->user ) );
}
/*
***************
*** 1277,1283 ****
*/
if ( p->authfile != NULL && checkauthfile ( p ) != 0 ) {
sleep ( SLEEP_SECONDS );
! return ( pop_msg ( p, POP_FAILURE, HERE, ERRMSG_AUTH, p->user ) );
}
/*
--- 1273,1279 ----
*/
if ( p->authfile != NULL && checkauthfile ( p ) != 0 ) {
sleep ( SLEEP_SECONDS );
! return ( pop_msg ( p, POP_FAILURE, HERE, ERRMSG_PW, p->user ) );
}
/*
*** pop_tls_openssl.c Fri Jun 1 20:24:35 2001
--- ../../qpopper4.0.2/popper/pop_tls_openssl.c Tue Apr 3 18:23:37 2001
***************
*** 5,14 ****
*
* Revisions:
*
- * 05/13/01 [rcg]
- * - Don't call SSL_shutdown unless we tried to negotiate an
- * SSL session. (As suggested by Kenneth Porter.)
- *
* 03/28/01 [rcg]
* - Don't log "OpenSSL Error during shutdown" unless compiled
* with debug or run with debug or trace options.
--- 5,10 ----
***************
*** 758,811 ****
int nErr = 0;
! if ( pTLS->m_pPOP->tls_started == TRUE ) {
! ret = SSL_shutdown ( pTLS->m_OpenSSLconn );
! DEBUG_LOG1 ( pTLS->m_pPOP, "tls shutdown returned %d", ret );
!
! nErr = SSL_get_error ( pTLS->m_OpenSSLconn, ret );
! DEBUG_LOG2 ( pTLS->m_pPOP, "SSL_get_error says %s (%d)",
! openssl_get_error_code(nErr), nErr );
! switch ( nErr ) {
! case SSL_ERROR_NONE:
! rslt = 0;
! break;
!
! case SSL_ERROR_ZERO_RETURN:
! rslt = -1;
! break;
!
! case SSL_ERROR_WANT_READ:
! case SSL_ERROR_WANT_WRITE:
! rslt = -1;
! break;
!
! case SSL_ERROR_WANT_X509_LOOKUP:
! rslt = -1;
! break;
!
! case SSL_ERROR_SYSCALL:
! rslt = -1;
! if ( DEBUGGING || pTLS->m_pPOP->debug )
! log_openssl_err ( pTLS->m_pPOP, HERE, "TLS shutdown Error" );
! break;
!
! case SSL_ERROR_SSL:
! rslt = -1;
! if ( DEBUGGING || pTLS->m_pPOP->debug )
! log_openssl_err ( pTLS->m_pPOP, HERE,
! "OpenSSL Error during shutdown" );
! break;
!
! default:
! rslt = -1;
log_openssl_err ( pTLS->m_pPOP, HERE,
! "Unknown error during shutdown" );
! break;
! } /* switch ( nErr ) */
! } /* pTLS->m_pPOP->tls_started == TRUE */
! else {
! DEBUG_LOG0 ( pTLS->m_pPOP, "pTLS->m_pPOP->tls_started == false" );
}
if ( pTLS->m_OpenSSLconn != NULL ) {
DEBUG_LOG0 ( pTLS->m_pPOP, "freeing m_OpenSSLconn" );
--- 754,803 ----
int nErr = 0;
! ret = SSL_shutdown ( pTLS->m_OpenSSLconn );
! DEBUG_LOG1 ( pTLS->m_pPOP, "tls shutdown returned %d", ret );
!
! nErr = SSL_get_error ( pTLS->m_OpenSSLconn, ret );
! DEBUG_LOG2 ( pTLS->m_pPOP, "SSL_get_error says %s (%d)",
! openssl_get_error_code(nErr), nErr );
! switch ( nErr ) {
! case SSL_ERROR_NONE:
! rslt = 0;
! break;
!
! case SSL_ERROR_ZERO_RETURN:
! rslt = -1;
! break;
!
! case SSL_ERROR_WANT_READ:
! case SSL_ERROR_WANT_WRITE:
! rslt = -1;
! break;
!
! case SSL_ERROR_WANT_X509_LOOKUP:
! rslt = -1;
! break;
!
! case SSL_ERROR_SYSCALL:
! rslt = -1;
! if ( DEBUGGING || pTLS->m_pPOP->debug )
! log_openssl_err ( pTLS->m_pPOP, HERE, "TLS shutdown Error" );
! break;
!
! case SSL_ERROR_SSL:
! rslt = -1;
! if ( DEBUGGING || pTLS->m_pPOP->debug )
log_openssl_err ( pTLS->m_pPOP, HERE,
! "OpenSSL Error during shutdown" );
! break;
!
! default:
! rslt = -1;
! log_openssl_err ( pTLS->m_pPOP, HERE,
! "Unknown error during shutdown" );
! break;
}
+
if ( pTLS->m_OpenSSLconn != NULL ) {
DEBUG_LOG0 ( pTLS->m_pPOP, "freeing m_OpenSSLconn" );
*** pop_user.c Sat Jun 2 00:21:14 2001
--- ../../qpopper4.0.2/popper/pop_user.c Tue Apr 3 18:23:39 2001
***************
*** 5,13 ****
*
* Revisions:
*
- * 06/01/01 [rcg]
- * - Fix buffer overflow (broken in 4.0b14).
- *
* 02/14/01 [rcg]
* - Fixed problems compiling with Kerberos 5.
*
--- 5,10 ----
***************
*** 197,207 ****
user_name_len = sizeof(p->user) -1;
}
! strlcpy ( p->user, p->pop_parm[1], sizeof(p->user) );
!
! #ifdef SCRAM
! strlcpy ( p->authid, p->user, sizeof(p->authid) ); /* userid is also authentication id */
! #endif /* SCRAM */
/*
* Cache passwd struct for use later; this memory gets freed at the end
--- 194,201 ----
user_name_len = sizeof(p->user) -1;
}
! strcpy ( p->user, p->pop_parm[1] );
! strcpy ( p->authid, p->user ); /* userid is also authentication id */
/*
* Cache passwd struct for use later; this memory gets freed at the end
*** pop_util.c Fri Jun 1 20:24:36 2001
--- ../../qpopper4.0.2/popper/pop_util.c Tue Apr 3 18:23:40 2001
***************
*** 7,15 ****
*
* Revisions:
*
- * 06/01/01 [rg]
- * - Fix from Arvin Schnell for warnings on 64-bit systems.
- *
* 04/02/01 [rg]
* - Changed macro STACKSIZE to QPSTACKSIZE to avoid conflicts.
*
--- 7,12 ----
***************
*** 38,51 ****
* Simple stack Implementation
* wish the compilers does inlining.
*/
! FP Push(CALLSTACK *s, FP f)
{
! return ( s->CurP < QPSTACKSIZE ) ? ( s->Stack[s->CurP++] = f ) : (FP) 0;
}
FP Pop(CALLSTACK *s)
{
! return s->CurP ? s->Stack[--s->CurP] : (FP) 0;
}
int StackSize(CALLSTACK *s)
--- 35,48 ----
* Simple stack Implementation
* wish the compilers does inlining.
*/
! int Push(CALLSTACK *s, FP f)
{
! return (int) ((s->CurP < QPSTACKSIZE) ? (s->Stack[s->CurP++] = f) : 0 );
}
FP Pop(CALLSTACK *s)
{
! return s->CurP ? s->Stack[--s->CurP] : (FP)0;
}
int StackSize(CALLSTACK *s)
*** popper.c Fri Jun 1 20:24:36 2001
--- ../../qpopper4.0.2/popper/popper.c Tue Apr 3 18:23:41 2001
***************
*** 15,24 ****
*
* Revisions:
*
- * 06/01/01 [rg]
- * - Added patch by Carles Xavier Munyoz to fix erroneous
- * scanning for \n in getline().
- *
* 01/15/01 [rg]
* - Handle run-time options bShy, bDo_timing, bUpdate_on_abort.
*
--- 15,20 ----
***************
*** 422,433 ****
/*
* Look for line in our buffer
*/
! for ( p = pPOP->pcInStart;
! p < pPOP->pcInEnd;
! p++ )
if ( *p == '\n' )
break;
! if ( p != pPOP->pcInEnd && *p == '\n' ) {
/*
* Got a line
*/
--- 418,428 ----
/*
* Look for line in our buffer
*/
! p = pPOP->pcInStart;
! for ( ; p < pPOP->pcInEnd; p++ )
if ( *p == '\n' )
break;
! if ( pPOP->pcInEnd != pPOP->pcInStart && *p == '\n' ) {
/*
* Got a line
*/
--x+6KMIRAuhnl3hBn--