Re: [Bug-wget] [PATCH v4] (resend) Add --use-askpass=COMMAND support

[Liam R. Howlett]

* Tim R?hsen <address@hidden> [160903 15:44]:
> Hi Liam,
> 
> thanks, we received the the FSF copyright assignment for Wget.
> 
> Can you give me an example of an external program to use with --use-askpass 
> (maybe a 'standard' one available on common Linux distros) ?

The most common utility to use for the askpass interface is
/usr/lib/ssh/x11-ssh-askpass
This is included in ssh-askpass in debian.

I have also tested with ksshaskpass along with just small c binaries to
dump long or short strings.


> 
> I still have trouble with:
> +  /* Set the end byte to \0, and decrement bytes */
> +  tmp[bytes--] = '\0';
> +
> +  /* Remove a possible new line */
> +  while (bytes >= 0 &&
> +        (tmp[bytes] == '\0' || tmp[bytes] == '\n' || tmp[bytes] == '\r'))
> +    tmp[bytes--] = '\0';
> +
> +  *answer = xmemdup (tmp, bytes + 2);
> 
> You introduce a buffer overflow by one byte here.


Ah, yes.  I've tested over 1024 and 1 character of '\n' and it works,
but I'm underflowing bytes then overflowing it back to the correct
value.

> 
> You could write instead e.g.:
> while (bytes > 0 &&
>     (tmp[bytes - 1] == '\0' || tmp[bytes - 1] == '\n' || tmp[bytes - 1] == 
> '\r'))
>   bytes--;
> 
> *answer = xmemdup0(tmp, bytes);
> 
> (needs to add xmemdup0 gnulib module to bootstrap.conf)
> 
> And if you do that, you can also:
>   bytes = read (com[0], tmp, sizeof (tmp));
> instead of
>   bytes = read (com[0], tmp, sizeof (tmp) - 1);

I am happy to change my code like you suggest above for clarity if you
are okay with adding xmemdup0 to bootstrap.conf.  I will resend v5 once
I retest with this update.

> 
> The patch looks fine otherwise ! Just have to test it with a program - do you 
> have something in mind ?

There are quite a few, debian & ubuntu set up an alternatives link for
ssh-askpass which can point to ksshaskpass, ssh-askpass-gnome,
ssh-askpass-fullscreen, and ssh-askpass.  ssh-askpass homepage is
http://www.jmknoble.net/software/x11-ssh-askpass/ according to apt-cache
show ssh-askpass, but the link seems dead.  Here is the debian.org
stable link: https://packages.debian.org/source/jessie/ssh-askpass
Since your mail client appears to be kmail, you could try ksshaskpass:
https://quickgit.kde.org/?p=ksshaskpass.git


Thanks,
Liam

> 
> Regards, Tim
> 
> 
> On Donnerstag, 1. September 2016 11:22:32 CEST Liam R. Howlett wrote:
> > This adds the --use-askpass option which is disabled by default.
> > 
> > --use-askpass=COMMAND will request the username and password for a given
> > URL by executing the external program COMMAND.  If COMMAND is left
> > blank, then the external program in the environment variable
> > WGET_ASKPASS will be used.  If WGET_ASKPASS is not set then the
> > environment variable SSH_ASKPASS is used.  If there is no value set, an
> > error is returned.  If an error occurs requesting the username or
> > password, wget will exit.
> > 
> > 
> > I am resending this patch because the FSF Contribution Agreement has been
> > signed by our legal team.
> > 
> > Liam R. Howlett (1):
> >   Add --use-askpass=COMMAND support
> > 
> >  bootstrap.conf |   1 +
> >  doc/wget.texi  |  17 ++++++---
> >  src/init.c     |  44 +++++++++++++++++++++++
> >  src/main.c     | 112
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/options.h  | 
> >  1 +
> >  src/url.c      |   6 ++++
> >  src/url.h      |   1 +
> >  7 files changed, 178 insertions(+), 4 deletions(-)
>