Skip to content

Commit

Permalink
ftp: understand --trust-server-names on a HTTP->FTP redirect
Browse files Browse the repository at this point in the history 
If not --trust-server-names is used, FTP will also get the destination
file name from the original url specified by the user instead of the
redirected url.  Closes CVE-2016-4971.

* src/ftp.c (ftp_get_listing): Add argument original_url.
(getftp): Likewise.
(ftp_loop_internal): Likewise.  Use original_url to generate the
file name if --trust-server-names is not provided.
(ftp_retrieve_glob): Likewise.
(ftp_loop): Likewise.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
  • Loading branch information
@giuseppe
giuseppe committed Jun 9, 2016
1 parent 2bdfc4f commit e996e32
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 30 deletions.
71 changes: 43 additions & 28 deletions src/ftp.c
View file
Expand Up @@ -236,7 +236,7 @@ print_length (wgint size, wgint start, bool authoritative)
logputs (LOG_VERBOSE, !authoritative ? _(" (unauthoritative)\n") : "\n");
}

static uerr_t ftp_get_listing (struct url *, ccon *, struct fileinfo **);
static uerr_t ftp_get_listing (struct url *, struct url *, ccon *, struct fileinfo **);

static uerr_t
get_ftp_greeting(int csock, ccon *con)
Expand Down Expand Up @@ -315,7 +315,8 @@ init_control_ssl_connection (int csock, struct url *u, bool *using_control_secur
and closes the control connection in case of error. If warc_tmp
is non-NULL, the downloaded data will be written there as well. */
static uerr_t
getftp (struct url *u, wgint passed_expected_bytes, wgint *qtyread,
getftp (struct url *u, struct url *original_url,
wgint passed_expected_bytes, wgint *qtyread,
wgint restval, ccon *con, int count, wgint *last_expected_bytes,
FILE *warc_tmp)
{
Expand Down Expand Up @@ -1188,7 +1189,7 @@ Error in server response, closing control connection.\n"));
{
bool exists = false;
struct fileinfo *f;
uerr_t _res = ftp_get_listing (u, con, &f);
uerr_t _res = ftp_get_listing (u, original_url, con, &f);
/* Set the DO_RETR command flag again, because it gets unset when
calling ftp_get_listing() and would otherwise cause an assertion
failure earlier on when this function gets repeatedly called
Expand Down Expand Up @@ -1779,8 +1780,8 @@ Error in server response, closing control connection.\n"));
This loop either gets commands from con, or (if ON_YOUR_OWN is
set), makes them up to retrieve the file given by the URL. */
static uerr_t
ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_file,
bool force_full_retrieve)
ftp_loop_internal (struct url *u, struct url *original_url, struct fileinfo *f,
ccon *con, char **local_file, bool force_full_retrieve)
{
int count, orig_lp;
wgint restval, len = 0, qtyread = 0;
Expand All @@ -1805,7 +1806,7 @@ ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_fi
{
/* URL-derived file. Consider "-O file" name. */
xfree (con->target);
con->target = url_file_name (u, NULL);
con->target = url_file_name (opt.trustservernames || !original_url ? u : original_url, NULL);
if (!opt.output_document)
locf = con->target;
else
Expand Down Expand Up @@ -1923,8 +1924,8 @@ ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_fi

/* If we are working on a WARC record, getftp should also write
to the warc_tmp file. */
err = getftp (u, len, &qtyread, restval, con, count, &last_expected_bytes,
warc_tmp);
err = getftp (u, original_url, len, &qtyread, restval, con, count,
&last_expected_bytes, warc_tmp);

if (con->csock == -1)
con->st &= ~DONE_CWD;
Expand Down Expand Up @@ -2092,7 +2093,8 @@ Removing file due to --delete-after in ftp_loop_internal():\n"));
/* Return the directory listing in a reusable format. The directory
is specifed in u->dir. */
static uerr_t
ftp_get_listing (struct url *u, ccon *con, struct fileinfo **f)
ftp_get_listing (struct url *u, struct url *original_url, ccon *con,
struct fileinfo **f)
{
uerr_t err;
char *uf; /* url file name */
Expand All @@ -2113,7 +2115,7 @@ ftp_get_listing (struct url *u, ccon *con, struct fileinfo **f)

con->target = xstrdup (lf);
xfree (lf);
err = ftp_loop_internal (u, NULL, con, NULL, false);
err = ftp_loop_internal (u, original_url, NULL, con, NULL, false);
lf = xstrdup (con->target);
xfree (con->target);
con->target = old_target;
Expand All @@ -2136,8 +2138,9 @@ ftp_get_listing (struct url *u, ccon *con, struct fileinfo **f)
return err;
}

static uerr_t ftp_retrieve_dirs (struct url *, struct fileinfo *, ccon *);
static uerr_t ftp_retrieve_glob (struct url *, ccon *, int);
static uerr_t ftp_retrieve_dirs (struct url *, struct url *,
struct fileinfo *, ccon *);
static uerr_t ftp_retrieve_glob (struct url *, struct url *, ccon *, int);
static struct fileinfo *delelement (struct fileinfo *, struct fileinfo **);
static void freefileinfo (struct fileinfo *f);

Expand All @@ -2149,7 +2152,8 @@ static void freefileinfo (struct fileinfo *f);
If opt.recursive is set, after all files have been retrieved,
ftp_retrieve_dirs will be called to retrieve the directories. */
static uerr_t
ftp_retrieve_list (struct url *u, struct fileinfo *f, ccon *con)
ftp_retrieve_list (struct url *u, struct url *original_url,
struct fileinfo *f, ccon *con)
{
static int depth = 0;
uerr_t err;
Expand Down Expand Up @@ -2310,7 +2314,10 @@ Already have correct symlink %s -> %s\n\n"),
else /* opt.retr_symlinks */
{
if (dlthis)
err = ftp_loop_internal (u, f, con, NULL, force_full_retrieve);
{
err = ftp_loop_internal (u, original_url, f, con, NULL,
force_full_retrieve);
}
} /* opt.retr_symlinks */
break;
case FT_DIRECTORY:
Expand All @@ -2321,7 +2328,10 @@ Already have correct symlink %s -> %s\n\n"),
case FT_PLAINFILE:
/* Call the retrieve loop. */
if (dlthis)
err = ftp_loop_internal (u, f, con, NULL, force_full_retrieve);
{
err = ftp_loop_internal (u, original_url, f, con, NULL,
force_full_retrieve);
}
break;
case FT_UNKNOWN:
logprintf (LOG_NOTQUIET, _("%s: unknown/unsupported file type.\n"),
Expand Down Expand Up @@ -2386,7 +2396,7 @@ Already have correct symlink %s -> %s\n\n"),
/* We do not want to call ftp_retrieve_dirs here */
if (opt.recursive &&
!(opt.reclevel != INFINITE_RECURSION && depth >= opt.reclevel))
err = ftp_retrieve_dirs (u, orig, con);
err = ftp_retrieve_dirs (u, original_url, orig, con);
else if (opt.recursive)
DEBUGP ((_("Will not retrieve dirs since depth is %d (max %d).\n"),
depth, opt.reclevel));
Expand All @@ -2399,7 +2409,8 @@ Already have correct symlink %s -> %s\n\n"),
ftp_retrieve_glob on each directory entry. The function knows
about excluded directories. */
static uerr_t
ftp_retrieve_dirs (struct url *u, struct fileinfo *f, ccon *con)
ftp_retrieve_dirs (struct url *u, struct url *original_url,
struct fileinfo *f, ccon *con)
{
char *container = NULL;
int container_size = 0;
Expand Down Expand Up @@ -2449,7 +2460,7 @@ Not descending to %s as it is excluded/not-included.\n"),
odir = xstrdup (u->dir); /* because url_set_dir will free
u->dir. */
url_set_dir (u, newdir);
ftp_retrieve_glob (u, con, GLOB_GETALL);
ftp_retrieve_glob (u, original_url, con, GLOB_GETALL);
url_set_dir (u, odir);
xfree (odir);

Expand Down Expand Up @@ -2508,14 +2519,15 @@ is_invalid_entry (struct fileinfo *f)
GLOB_GLOBALL, use globbing; if it's GLOB_GETALL, download the whole
directory. */
static uerr_t
ftp_retrieve_glob (struct url *u, ccon *con, int action)
ftp_retrieve_glob (struct url *u, struct url *original_url,
ccon *con, int action)
{
struct fileinfo *f, *start;
uerr_t res;

con->cmd |= LEAVE_PENDING;

res = ftp_get_listing (u, con, &start);
res = ftp_get_listing (u, original_url, con, &start);
if (res != RETROK)
return res;
/* First: weed out that do not conform the global rules given in
Expand Down Expand Up @@ -2611,7 +2623,7 @@ ftp_retrieve_glob (struct url *u, ccon *con, int action)
if (start)
{
/* Just get everything. */
res = ftp_retrieve_list (u, start, con);
res = ftp_retrieve_list (u, original_url, start, con);
}
else
{
Expand All @@ -2627,7 +2639,7 @@ ftp_retrieve_glob (struct url *u, ccon *con, int action)
{
/* Let's try retrieving it anyway. */
con->st |= ON_YOUR_OWN;
res = ftp_loop_internal (u, NULL, con, NULL, false);
res = ftp_loop_internal (u, original_url, NULL, con, NULL, false);
return res;
}

Expand All @@ -2647,8 +2659,8 @@ ftp_retrieve_glob (struct url *u, ccon *con, int action)
of URL. Inherently, its capabilities are limited on what can be
encoded into a URL. */
uerr_t
ftp_loop (struct url *u, char **local_file, int *dt, struct url *proxy,
bool recursive, bool glob)
ftp_loop (struct url *u, struct url *original_url, char **local_file, int *dt,
struct url *proxy, bool recursive, bool glob)
{
ccon con; /* FTP connection */
uerr_t res;
Expand All @@ -2669,16 +2681,17 @@ ftp_loop (struct url *u, char **local_file, int *dt, struct url *proxy,
if (!*u->file && !recursive)
{
struct fileinfo *f;
res = ftp_get_listing (u, &con, &f);
res = ftp_get_listing (u, original_url, &con, &f);

if (res == RETROK)
{
if (opt.htmlify && !opt.spider)
{
struct url *url_file = opt.trustservernames ? u : original_url;
char *filename = (opt.output_document
? xstrdup (opt.output_document)
: (con.target ? xstrdup (con.target)
: url_file_name (u, NULL)));
: url_file_name (url_file, NULL)));
res = ftp_index (filename, u, f);
if (res == FTPOK && opt.verbose)
{
Expand Down Expand Up @@ -2723,11 +2736,13 @@ ftp_loop (struct url *u, char **local_file, int *dt, struct url *proxy,
/* ftp_retrieve_glob is a catch-all function that gets called
if we need globbing, time-stamping, recursion or preserve
permissions. Its third argument is just what we really need. */
res = ftp_retrieve_glob (u, &con,
res = ftp_retrieve_glob (u, original_url, &con,
ispattern ? GLOB_GLOBALL : GLOB_GETONE);
}
else
res = ftp_loop_internal (u, NULL, &con, local_file, false);
{
res = ftp_loop_internal (u, original_url, NULL, &con, local_file, false);
}
}
if (res == FTPOK)
res = RETROK;
Expand Down
3 changes: 2 additions & 1 deletion src/ftp.h
View file
Expand Up @@ -169,7 +169,8 @@ enum wget_ftp_fstatus
};

struct fileinfo *ftp_parse_ls (const char *, const enum stype);
uerr_t ftp_loop (struct url *, char **, int *, struct url *, bool, bool);
uerr_t ftp_loop (struct url *, struct url *, char **, int *, struct url *,
bool, bool);

uerr_t ftp_index (const char *, struct url *, struct fileinfo *);

Expand Down
3 changes: 2 additions & 1 deletion src/retr.c
View file
Expand Up @@ -830,7 +830,8 @@ retrieve_url (struct url * orig_parsed, const char *origurl, char **file,
if (redirection_count)
oldrec = glob = false;

result = ftp_loop (u, &local_file, dt, proxy_url, recursive, glob);
result = ftp_loop (u, orig_parsed, &local_file, dt, proxy_url,
recursive, glob);
recursive = oldrec;

/* There is a possibility of having HTTP being redirected to
Expand Down

0 comments on commit e996e32

Please sign in to comment.