Bug 131088 - mc shows ftp password in plain text
mc shows ftp password in plain text
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: mc (Show other bugs)
3
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jindrich Novy
: Security
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2004-08-27 09:45 EDT by George Toft
Modified: 2013-07-02 19:01 EDT (History)
3 users (show)

See Also:
Fixed In Version: 4.6.1-0.9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2004-11-08 07:57:51 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
This patch prevents mc displaying user's ftp password when uploading. (766 bytes, text/plain)
2004-09-08 09:05 EDT, Jindrich Novy
no flags Details
This patch prevents mc displaying user's ftp password when uploading. (766 bytes, patch)
2004-09-08 09:06 EDT, Jindrich Novy
no flags Details | Diff
The patch tells mc not to display password in path in copy dialog, delete, error dialogs, etc. (6.91 KB, patch)
2004-09-14 10:10 EDT, Jindrich Novy
no flags Details | Diff
the improved version of strippwd patch (9.79 KB, patch)
2004-10-08 09:06 EDT, Jindrich Novy
no flags Details | Diff
add_new_entry_cmd() without memory leaks. (1.10 KB, patch)
2004-10-15 12:44 EDT, Andrew V. Samoilov
no flags Details | Diff
the strippwd patch with the fix of possible memleak (10.55 KB, patch)
2004-10-27 11:51 EDT, Jindrich Novy
no flags Details | Diff

  None (edit)
Description George Toft 2004-08-27 09:45:01 EDT
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040625

Description of problem:
While performing FTP upload to remote server, the remote credentials
are displayed in clear text.

Version-Release number of selected component (if applicable):


How reproducible:
Always

Steps to Reproduce:
1.  Launch mc
2.  [F9] - select "FTP link..."
3.  Enter username:password@remotehost information
4.  Select file to upload
5.  View remote host credentials
    

Actual Results:  This was taken during an actual upload.  The filename
and credentials have been changed and are not valid.

################################### Copy
###################################
#                                                                    
     #
#   Source   msjavx.html                                             
     #                               
#   Target  
/#ftp:joeuser:mypassword@ftp.georgetoft.com/FTP/msjavx.html   #
#                                                                    
     #
#   File     [                                        ]  62% ETA
0:00.45   #
#   Bytes    [                                        ]  81% 43.83
KB/s    #
#   Count    [                                        ]  67%         
     #
#                                                                    
     #
#                  [ Skip ]                     [ Abort ]            
     #
#                                                                    
     #
############################################################################


Expected Results:  The remote host password (and preferably the
username) should NOT be displayed.

Additional info:
Comment 1 Jindrich Novy 2004-09-08 09:03:21 EDT
Hi George, the following patch should help you to get rid of this issue.

Jindrich
Comment 2 Jindrich Novy 2004-09-08 09:05:51 EDT
Created attachment 103581 [details]
This patch prevents mc displaying user's ftp password when uploading.

This patch prevents mc displaying user's ftp password when uploading.
Comment 3 Jindrich Novy 2004-09-08 09:06:00 EDT
Created attachment 103582 [details]
This patch prevents mc displaying user's ftp password when uploading.

This patch prevents mc displaying user's ftp password when uploading.
Comment 4 Jindrich Novy 2004-09-14 10:10:34 EDT
Created attachment 103826 [details]
The patch tells mc not to display password in path in copy dialog, delete, error dialogs, etc.

The patch is tested, please apply.
Comment 5 Leonard den Ottolander 2004-10-05 20:54:11 EDT
Could you also get a patch like this for the directory hotlist?
Comment 6 Jindrich Novy 2004-10-06 06:53:12 EDT
I improved the patch a little bit and consulted it with Jakub.
Ok, let me check it.
Comment 7 Leonard den Ottolander 2004-10-06 13:15:02 EDT
Jindrich, could you propose these patches on the mc-devel at gnome dot
org list? That's probably easier. I've already started a thread on the
issue.
Comment 8 Leonard den Ottolander 2004-10-06 13:32:57 EDT
Please mark patches as obsolete when appropriate.
Comment 9 Leonard den Ottolander 2004-10-06 13:40:50 EDT
Please compare
http://mail.gnome.org/archives/mc-devel/2004-August/msg00167.html .
Comment 10 Andrew V. Samoilov 2004-10-06 19:46:44 EDT
strip_password_only() can write to const *char path in this
implementation.  It can be fixed:

new_path = g_strdup_printf ("%.*s%s", at - col, path, at);

Also, upstream supports username with @ in it, so this function has to
be reworked to check against last @ before /.
Comment 11 Jindrich Novy 2004-10-07 04:59:43 EDT
Andrew, thanks for the workaround.

Leonard, that's a good idea, I'll play with the patches a bit more and
do the proposal.
Comment 12 Jindrich Novy 2004-10-08 09:03:45 EDT
The improved version of the .strippwd patch fixes showing passwords in
hotlist and error messages in case chdir fails in subshell, etc.

With Jakub's help I modified strip_password in the way that
strip_password_only () is no more needed.

Leonard, the version of strip_passwd () is different from fix
mentioned in mc-devel. With Jakub we concluded that our implementation
that doesn't modify the input path at all is a bit better.

Comment 13 Jindrich Novy 2004-10-08 09:06:18 EDT
Created attachment 104935 [details]
the improved version of strippwd patch

Note that it uses mbstrlen () instead of strlen ().
Comment 14 Andrew V. Samoilov 2004-10-12 11:31:22 EDT
Memory leaks, look for label_string.
Also it seems there is one more typo in util.c: s/Cavevat/Caveat/
BTW, can you consult us about use of functions from GPLed src/util.c
in the LGPLed vfs/*.c ? Is it legal?
Comment 15 Jindrich Novy 2004-10-15 08:38:16 EDT
Hi Andrew,

I see no memory leak in function add2hotlist_cmd () regarding
label_string. name_trunc () prints to static string so there's no need
to g_free(). There's also no problem with input_dialog ().
label_string is not g_free()d because it's done so in remove_group ()
called by done_hotlist (). If I missed something, can you please
specify where exactly is the leak? 

Well, the vfs/*.c is licensed under LGPL because it used gnome-vfs
some time ago but as I had a discussion on the topic with Jakub,
there's no legal problem to use LGPLed source from GPLed programs. The
license may change back to GPL if all the authors agree, what is
rather rare case...
Comment 16 Andrew V. Samoilov 2004-10-15 10:29:21 EDT
Hi, Jindrich

label_string is leaked if input_dialog() returns NULL or empty string.
In last case this memory (g_strdup("")) is also leaked.

About our legal issue: we have reverse case.  Now LGPLed VFS sources use
GPLed functions from src/util.c and Roland Illig points this as a problem.
Comment 17 Andrew V. Samoilov 2004-10-15 12:44:59 EDT
Created attachment 105280 [details]
add_new_entry_cmd() without memory leaks.

quick_dialog() and so add_new_entry_cmd() returns malloc()ed title and url.
BTW platform should be changed to All.
Comment 18 Leonard den Ottolander 2004-10-27 09:06:55 EDT
Jindrich, can we fix this quickly so it can be committed before the
release of 4.6.1? TIA.
Comment 19 Leonard den Ottolander 2004-10-27 09:13:12 EDT
Please see http://www.pavelsh.pp.ru/wiki/doku.php?id=mc-bugs , "hide
FTP password in various places" for references to existing patches.
Comment 20 Jindrich Novy 2004-10-27 10:05:57 EDT
Leonard, sure, I'll have look at it.
Comment 21 Jindrich Novy 2004-10-27 11:51:21 EDT
Created attachment 105847 [details]
the strippwd patch with the fix of possible memleak

So finally I had a time to fix the possible memory leak Andrew pointed out.

Andrew, have you posted your add_new_entry_cmd() patch also to mc-devel?
Comment 22 Jindrich Novy 2004-11-08 07:57:51 EST
Hi,

The significant part of the patch was applied upstream and
mc-4.6.1-0.9 shouldn't have problems with showing ftp passwords in
URLs any more.

Note You need to log in before you can comment on or make changes to this bug.