38300 - Two minor fixes for nsFtpConnectionThread.cpp

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Wan-Teh Chang]

I'm going to attach a patch that contains two fixes
for mozilla/netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp.

1. In nsFtpConnectionThread::R_pasv(), we have
    PRInt32 h0, h1, h2, h3, p0, p1;
    ...
    PRInt32 fields = PR_sscanf(ptr,
#ifdef __alpha
        "%d,%d,%d,%d,%d,%d",
#else
        "%ld,%ld,%ld,%ld,%ld,%ld",
#endif
        &h0, &h1, &h2, &h3, &p0, &p1);

  The #ifdef __alpha is not needed.  The %ld format
  for PR_snprintf and PR_sscanf specifies PRInt32, not
  long.  I checked our documentation
  (http://www.mozilla.org/docs/refList/refNSPR/prprf.html)
  and found that this is not clearly documented.
      %hd   PRInt16
      %d    PRIntn
      %ld   PRInt32
      %lld  PRInt64

2. In nsFtpConnectionThread::R_pasv(), the while loop
  to find the beginning of the returned address string
  is not robust against a malformed response.
    // The returned address string can be of the form
    // (xxx,xxx,xxx,xxx,ppp,ppp) or
    //  xxx,xxx,xxx,xxx,ppp,ppp (without parens)
    ptr = response;
    while (*ptr++) {
        if (*ptr == '(') {
            // move just beyond the open paren
            ptr++;
            break;
        }
        if (*ptr == ',') {
            ptr--;
            // backup to the start of the digits
            while ( (*ptr >= '0') && (*ptr <= '9'))
                ptr--;
            ptr++; // get back onto the numbers
            break;
        }
    } // end while

  The ++ in 'while (*ptr++)' will move 'ptr' beyond
  the terminating null byte if 'response' does not
  contain any '(' and ','.

  The inner while loop, which moves 'ptr' backward,
  needs to check for (ptr >= response) to prevent
  'ptr' from moving past the beginning of 'response'.
  A problematic malformed 'response' for this would
  be "9,9".

Copied jgmyers because this is in the area that his
IPv6 FTP patch touches (bug #28874).

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch.

Comment 2

[John G. Myers]

The conflict between this patch and bug #28874 is trivial to resolve--it's just 
a difference of indentation.

Comment 3

[Gagan]

will check this in tonite. thx wtc.

Comment 4

[Gagan]

checked in. 

Comment 5

[Wan-Teh Chang]

I verified the fix.  Thanks Gagan.