Re: strip_passwd partial commit

[Jindrich Novy <jnovy redhat com>]

Hello Pavel,

On Thu, 2004-11-04 at 06:47, Pavel Shirshov (pchel) wrote:
>      I didn't patch:
> 
>        hotlist.c - need some testing and improvements. I want to save
>        full url (with password), but display title without password.

Note that it would introduce a possible security issue to mc. Because of
the fact we're saving directory hotlist to ~/.mc/hotlist, superuser
could read for instance ftp URLs with passwords since he has access to
home directories of all the users.

OTOH the next possible solution is to keep the URLs in ~/.mc/hotlist
with passwords stripped and prompt an user for password first time, keep
it in memory and for later connections simply add the password to URL
from memory so an user needn't to type the password again.

In my opinion the only secure solution is not to save URLs with
passwords at all. So I decided to not implement such feature in the
patch.

>        util.c(strip_password()) - need some additional testing. For
>        example around:
> -        at = strrchr (p, '@');
> +       at = memchr (p, '@', host_len);
> 
>        It's not equal operations.

Yes, they're not equal, because I changed the password stripping
strategy (see also the condition above this line), because of this bug
in strip_password ():

It strips not only the password but everything after first PATH_SEP, so:

jamesbond:007 ci5 uk/home/secret/

is after strip_password ():

jamesbond ci5 uk

what is also incorrectly written to histories, where the directory on
ci5.uk is simply cut out. It's because of the fact that after:

                strcpy (inner_colon, at);

dir points somewhere else than before, so an attempt to return the '/'
to its original position:

        if (dir)
            *dir = PATH_SEP;

of course fails.

This is fixed in the implementation presented in the patch, furthermore
with using memchr/memmove functions we needn't to write any chars into
the char *p, which is also the argument of strip_password () and thus no
such bug could occur again. [tested]

greetings,
Jindrich

-- 
Jindrich Novy <jnovy redhat com>, http://people.redhat.com/jnovy/