On Fri, Sep 11, 2009 at 10:51:11PM +0200, Ruediger Pluem wrote:
> On 09/11/2009 04:52 PM, Joe Orton wrote:
> > On Thu, Sep 10, 2009 at 07:02:01PM +0200, Stefan Fritsch wrote:
> >> The (untested) patch below should fix CVE-2009-3094. For CVE-2009-3095
> >> there is only little information. But looking at the code, it seems
> >> the username and password sent by the browser are sent to the ftp
> >> server without sanitization (i.e. they can contain LF characters).
> >
> > Spot on, good catch.
> >
> > using "Authorization: Basic RkVBVApGRUFUCg==" results in:
> >
> > read(11, "220 (vsFTPd 2.0.6)\r\n", 8000) = 20
> > write(2, "[Fri Sep 11 15:46:18 2009] [debu"..., 88) = 88
> > writev(11, [{"USER FEAT\nFEAT\n\r\n", 17}], 1) = 17
> > write(2, "[Fri Sep 11 15:46:18 2009] [debu"..., 87) = 87
> >
> > I think this should be sufficient - any other characters it's worth
> > filtering for?
>
> Seems sufficient to me. Don't we need to this as well a few lines
> below for username and password fetched from the URL?
...
> Couldn't the username / password part of the URL contain %0A / %OD?
It seems that the code already catches that case -
proxy_ftp_canon extracts username/password and does:
if (user != NULL && !ftp_check_string(user))
return HTTP_BAD_REQUEST;
if (password != NULL && !ftp_check_string(password))
return HTTP_BAD_REQUEST;
where ftp_check_string() checks specifically for CR/LF.
I hadn't spotted ftp_check_string() before but clearly that should be
used for the check here too - I'll adjust the patch and commit that part
since it's less complicated than the other issue.
Regards, Joe
