[20879] in bugtraq

home help back first fref pref prev next nref lref last post

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--

home help back first fref pref prev next nref lref last post