Re: [Bug-wget] [PATCH v2] bug #48811: netrc password wins over interactive --ask-password

[Tim Rühsen]

Hi Piotr,

please include netrc.h in utils.c.

In getftp(): 
  struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
...
  ftp_cred = pick_credentials()

This looks like a memleak. Also, where do you free ftp_cred ?
Is it really necessary  to allocate ftp_cred on each call to getftp (just a 
question, maybe it is) ?

Basically the same in gethttp()...

Try 'make check-valgrind' to test for some kinds of memleaks.

Regards, Tim

On Mittwoch, 19. Oktober 2016 11:53:22 CEST losgrandes wrote:
> * src/ftp.c: Leverage new struct net_credentials and function
> pick_credentials. pick_credentials is responsible for taking proper order
> when selecting source of credentials. * src/http.c: Leverage new struct
> net_credentials and function pick_credentials. * src/utils.c: New function
> pick_credentials.
> * src/utils.h: New struct net_credentials.
> 
> ---
>  src/ftp.c   | 19 +++++++------------
>  src/http.c  | 18 +++++++-----------
>  src/utils.c | 19 +++++++++++++++++++
>  src/utils.h | 11 +++++++++++
>  4 files changed, 44 insertions(+), 23 deletions(-)
> 
> diff --git a/src/ftp.c b/src/ftp.c
> index 39f20fa..cc98ca3 100644
> --- a/src/ftp.c
> +++ b/src/ftp.c
> @@ -327,7 +327,8 @@ getftp (struct url *u, struct url *original_url,
>    uerr_t err = RETROK;          /* appease the compiler */
>    FILE *fp = NULL;
>    char *respline, *tms;
> -  const char *user, *passwd, *tmrate;
> +  const char *tmrate;
> +  struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
>    int cmd = con->cmd;
>    wgint expected_bytes = 0;
>    bool got_expected_bytes = false;
> @@ -359,13 +360,7 @@ getftp (struct url *u, struct url *original_url,
> 
>    *qtyread = restval;
> 
> -  user = u->user;
> -  passwd = u->passwd;
> -  search_netrc (u->host, (const char **)&user, (const char **)&passwd, 1);
> -  user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
> -  if (!user) user = "anonymous";
> -  passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd :
> opt.passwd); -  if (!passwd) passwd = "-wget@";
> +  ftp_cred = pick_credentials (u, opt.ftp_user, opt.ftp_passwd, opt.user,
> opt.passwd, 1);
> 
>    dtsock = -1;
>    local_sock = -1;
> @@ -461,18 +456,18 @@ getftp (struct url *u, struct url *original_url,
> 
>        /* Second: Login with proper USER/PASS sequence.  */
>        logprintf (LOG_VERBOSE, _("Logging in as %s ... "),
> -                 quotearg_style (escape_quoting_style, user));
> +                 quotearg_style (escape_quoting_style, ftp_cred->user));
>        if (opt.server_response)
>          logputs (LOG_ALWAYS, "\n");
>        if (con->proxy)
>          {
>            /* If proxy is in use, log in as address@hidden */
> -          char *logname = concat_strings (user, "@", u->host, (char *) 0);
> -          err = ftp_login (csock, logname, passwd);
> +          char *logname = concat_strings (ftp_cred->user, "@", u->host,
> (char *) 0); +          err = ftp_login (csock, logname, ftp_cred->passwd);
>            xfree (logname);
>          }
>        else
> -        err = ftp_login (csock, user, passwd);
> +        err = ftp_login (csock, ftp_cred->user, ftp_cred->passwd);
> 
>        /* FTPRERR, FTPSRVERR, WRITEFAILED, FTPLOGREFUSED, FTPLOGINC */
>        switch (err)
> diff --git a/src/http.c b/src/http.c
> index 7e2c4ec..41eaa42 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -1813,7 +1813,7 @@ time_to_rfc1123 (time_t time, char *buf, size_t
> bufsize) static struct request *
>  initialize_request (const struct url *u, struct http_stat *hs, int *dt,
> struct url *proxy, bool inhibit_keep_alive, bool *basic_auth_finished, -   
>                 wgint *body_data_size, char **user, char **passwd, uerr_t
> *ret) +                    wgint *body_data_size, struct net_credentials
> **http_cred, uerr_t *ret) {
>    bool head_only = !!(*dt & HEAD_ONLY);
>    struct request *req;
> @@ -1876,20 +1876,16 @@ initialize_request (const struct url *u, struct
> http_stat *hs, int *dt, struct u request_set_header (req,
> "Accept-Encoding", "identity", rel_none);
> 
>    /* Find the username and password for authentication. */
> -  *user = u->user;
> -  *passwd = u->passwd;
> -  search_netrc (u->host, (const char **)user, (const char **)passwd, 0);
> -  *user = *user ? *user : (opt.http_user ? opt.http_user : opt.user);
> -  *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd :
> opt.passwd); +  *http_cred = pick_credentials (u, opt.http_user,
> opt.http_passwd, opt.user, opt.passwd, 0);
> 
>    /* We only do "site-wide" authentication with "global" user/password
>     * values unless --auth-no-challange has been requested; URL
> user/password * info overrides. */
> -  if (*user && *passwd && (!u->user || opt.auth_without_challenge))
> +  if ((*http_cred)->user && (*http_cred)->passwd && (!u->user ||
> opt.auth_without_challenge)) {
>        /* If this is a host for which we've already received a Basic
>         * challenge, we'll go ahead and send Basic authentication creds. */
> -      *basic_auth_finished = maybe_send_basic_creds (u->host, *user,
> *passwd, req); +      *basic_auth_finished = maybe_send_basic_creds
> (u->host, (*http_cred)->user, (*http_cred)->passwd, req); }
> 
>    /* Generate the Host header, HOST:PORT.  Take into account that:
> @@ -2920,7 +2916,7 @@ gethttp (const struct url *u, struct url
> *original_url, struct http_stat *hs, struct request *req = NULL;
> 
>    char *type = NULL;
> -  char *user, *passwd;
> +  struct net_credentials *http_cred = malloc (sizeof *http_cred);
>    char *proxyauth;
>    int statcode;
>    int write_error;
> @@ -3029,7 +3025,7 @@ gethttp (const struct url *u, struct url
> *original_url, struct http_stat *hs, uerr_t ret;
>      req = initialize_request (u, hs, dt, proxy, inhibit_keep_alive,
>                                &basic_auth_finished, &body_data_size,
> -                              &user, &passwd, &ret);
> +                              &http_cred, &ret);
>      if (req == NULL)
>        {
>          retval = ret;
> @@ -3360,7 +3356,7 @@ gethttp (const struct url *u, struct url
> *original_url, struct http_stat *hs, pconn.authorized = false;
> 
>        {
> -        auth_err = check_auth (u, user, passwd, resp, req,
> +        auth_err = check_auth (u, http_cred->user, http_cred->passwd, resp,
> req, &ntlm_seen, &retry,
>                                 &basic_auth_finished,
>                                 &auth_finished);
> diff --git a/src/utils.c b/src/utils.c
> index 9ab1b90..ed063cf 100644
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -440,6 +440,25 @@ fmttime (time_t t, const char *fmt)
>    return output;
>  }
> 
> +/*
> + Pick all credentials provided using --user, --password, --http-user,
> --http-password, --ftp-user, --ftp-password, --ask-password and from .netrc
> file. Select most important ones. + */
> +struct net_credentials *
> +pick_credentials (const struct url *u, char *protocol_specific_user, char
> *protocol_specific_passwd, char *opt_user, char *opt_passwd, int
> slack_default) { +  struct net_credentials *cred = malloc (sizeof *cred);
> +  cred->user = u->user;
> +  cred->user = cred->user ? cred->user : (protocol_specific_user ?
> protocol_specific_user : opt.user); +  cred->passwd = opt_passwd ?
> opt_passwd : (u->passwd ? u->passwd : protocol_specific_passwd); +  if
> (!cred->user && !cred->passwd) search_netrc (u->host, (const char
> **)&cred->user, (const char **)&cred->passwd, slack_default); +
> +  if (slack_default) {
> +    if (!cred->user) cred->user = "anonymous";
> +    if (!cred->passwd) cred->passwd = "-wget@";
> +  }
> +
> +  return cred;
> +}
> +
>  /* Return pointer to a static char[] buffer in which zero-terminated
>     string-representation of TM (in form hh:mm:ss) is printed.
> 
> diff --git a/src/utils.h b/src/utils.h
> index f224b73..17fb845 100644
> --- a/src/utils.h
> +++ b/src/utils.h
> @@ -29,6 +29,9 @@ Corresponding Source for a non-source form of such a
> combination shall include the source code for the parts of OpenSSL used as
> well as that of the covered work.  */
> 
> +// Include this to handle 'const struct url *' in pick_credentials()
> +#include "url.h"
> +
>  #ifndef UTILS_H
>  #define UTILS_H
> 
> @@ -63,6 +66,14 @@ struct file_memory {
>    int mmap_p;
>  };
> 
> +/* Struct and function for handling HTTP and FTP credentials */
> +struct net_credentials {
> +    char *user;
> +    char *passwd;
> +};
> +
> +struct net_credentials * pick_credentials (const struct url *, char *, char
> *, char *, char *, int); +
>  #define HYPHENP(x) (*(x) == '-' && !*((x) + 1))
> 
>  char *time_str (time_t);

signature.asc
Description: This is a digitally signed message part.