BUG REPORT: module popper.c, function getline()

[Carles Xavier Munyoz Bald�]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,
I'm using Qpopper 4.0.2 and I have found a bug in the module 'popper.c', 
function 'getline()'.

The bug is in the line:
425:     if ( pPOP->pcInEnd != pPOP->pcInStart && *p == '\n' ) {

The solution is modify the line this way:
OLD 425:      if ( pPOP->pcInEnd != pPOP->pcInStart && *p == '\n' ) {
NEW 425:     if ( p != pPOP->pcInEnd && *p == '\n' ) {


I'm going to explain the bug.
Its something hard to explain, but I'll do my best :-)

The problem appears when I try to test one of my pop users accounts using the 
telnet client from Windows.
This telnet client sends one TCP packet for every keystroke.
The function getline stores one line from the client TCP connection in one 
buffer. If this BUFFER IS NOT INITIALIZED it will contain RANDOM BYTES.
Imagin that the buffer has this random bytes:                               
    --------------------
    | a |   | g | 1 |   | F | k |\n | X | ....
    --------------------

and that I type the line 'pass pass00' in my Windows telnet client.

When Qpopper calls the function getline() the status of the buffer and the 
pointers will be:

    --------------------
    | a |   | g | 1 |   | F | k |\n | X | ....
    --------------------
      ^------pPOP->pcInStart
      ^------pPOP->pcInEnd
      ^------p

After the first read system call over the connected socket:
526:        nRead = read ( pPOP->input_fd,  pPOP->pcInEnd, nRoom );
the status of the buffer and the pointers will be:
    --------------------
    | p |   | g | 1 |   | F | k |\n | X | ....
    --------------------
      ^--------pPOP->pcInStart
          ^------pPOP->pcInEnd
      ^--------p

Here the Qpopper process has readed the first letter of the pass keyword from 
the connected socket. Due that my telnet client sends one TCP packet for 
every keystroke, when Qpopper issues the read system call over the socket, it 
will read only one byte.

Next the program flow goes to the begining of the main while loop of the 
getline() function and this is the code executed:
418:        /*
419:         * Look for line in our buffer
420:         */
421:        p = pPOP->pcInStart;
422:        for ( ; p < pPOP->pcInEnd; p++ )
423:            if ( *p == '\n' )
424:                break;

After this loop the status will be:
    --------------------
    | p |   | g | 1 |   | F | k |\n | X | ....
    --------------------
      ^--------pPOP->pcInStart
          ^------pPOP->pcInEnd
          ^------p

And then the code line in which the bug is will be executed:
OLD 425:        if ( pPOP->pcInEnd != pPOP->pcInStart && *p == '\n' ) {
At this point we we can see the bug.
If you see the status of the p pointer,  its POINTING A BYTE WITH RANDOM DATA 
and we are using it as part of the if conditions of the code line with the 
bug !!!!!!

If we follow the program, we will reach the status:
    --------------------
    | p | a | s | s |   | p | a |\n | X | ....
    --------------------
      ^--- -----------------pPOP->pcInStart
                                      ^------pPOP->pcInEnd
                                      ^------p

In this status the conditios for the if stament in the line 425:
OLD 425:        if ( pPOP->pcInEnd != pPOP->pcInStart && *p == '\n' ) {
become both true, making that the getline() function finish returning the 
line 'pass pa', instead of the line I typed 'pass pass00', and my Qpopper 
finish giving the error '-ERR [AUTH] Password supplied for "user" is 
incorrect.'.

How I have said, the solution to the bug is very simple, modify the line 425 
this way:
OLD 425:      if ( pPOP->pcInEnd != pPOP->pcInStart && *p == '\n' ) {
NEW 425:     if ( p != pPOP->pcInEnd && *p == '\n' ) {

With this modification, if the p pointer points to the same random byte that 
pPOP->pcInEnd, the first condition of the if stament is not true and the 
function getline() continues until a '\n' caracter is read from the socket.
  
I have observed that the bug only appears with users with the length of its 
password string greater than the length of its user name. I believe that this 
is because the same buffer is used to store the POP request 'user username', 
and the next 'pass password', but the buffer is not cleaned before the 
receive of the pass POP request.

This problem is difficult to see because the email clients like Netscape, 
Outlook, ... sends all the request in one TCP packet and when Qpopper calls 
the read system call in the getline() function, it obtains all the bytes from 
the request, including the '\n' byte. This way the bug never appears because 
the getline function reads the '\n' byte in the request and, then, the p 
pointer will never point to random data.

UFffff, I hope that my explanation will be clear enougth :-)


Well, this is my second bug report for the Qpopper development team :-)
Do you have any award or prize for acumulation of bug reporting ?????  ;-)

Greetings.
- ---
Carles Xavier Munyoz Bald� / [EMAIL PROTECTED]
VAS - Experto en Sistemas IP
Wanadoo Espa�a - http://www.wanadoo.es/
Tel: +34 96 5040046 - Fax: +34 96 5040047
- ---
-----BEGIN PGP SIGNATURE-----
Version: PGP 6.5.8

iQA/AwUBOwVGPRAGkoZz8//aEQLqogCfUaLHMHGA5cAIkApWPSUGEGUqpgMAnR9M
ZkgTgFEYrspPYhOzkykdMmQs
=2Pyg
-----END PGP SIGNATURE-----